Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use max_weight_to_satisfy instead of max_satisfaction_weight #1036

Closed
Tracked by #72 ...
danielabrozzoni opened this issue Jul 19, 2023 · 3 comments · Fixed by #1345
Closed
Tracked by #72 ...

Use max_weight_to_satisfy instead of max_satisfaction_weight #1036

danielabrozzoni opened this issue Jul 19, 2023 · 3 comments · Fixed by #1345
Assignees
Labels
api A breaking API change module-wallet
Milestone

Comments

@danielabrozzoni
Copy link
Member

danielabrozzoni commented Jul 19, 2023

Needs #1023

max_satisfaction_weight is deprecated, we should switch to using max_weight_to_satisfy instead.

Note that the switch is not an easy search and replace, the two methods have slightly different definitions

evanlinjin added a commit that referenced this issue Aug 4, 2023
1da3b30 ci: Pin rustls to keep the MSRV (Daniela Brozzoni)
792b39f Explicitly deny multipath keys (Daniela Brozzoni)
b73385d Update wallet_electrum to rust-bitcoin 0.30.0 (Daniela Brozzoni)
3dac3f9 Update example_electrum to rust-bitcoin 0.30.0 (Daniela Brozzoni)
2949bdc Update example_cli to rust-bitcoin 0.30.0 (Daniela Brozzoni)
468d2a0 Update tmp_plan to rust-bitcoin 0.30.0 (Daniela Brozzoni)
b8ac16d Update coin_select to rust-bitcoin 0.30.0 (Daniela Brozzoni)
6c29e53 Update wallet_esplora and wallet_esplora_async to... ...rust-bitcoin 0.30.0 (Daniela Brozzoni)
6eb0795 Update crates/esplora to rust-bitcoin 0.30.0 (Daniela Brozzoni)
91b0f0b Update crates/electrum to bitcoin 0.30.0 (Daniela Brozzoni)
f4e3ba3 Update bdk to bitcoin 0.30.0 (Daniela Brozzoni)
853d361 Update bdk_chain to bitcoin 0.30.0 (Daniela Brozzoni)

Pull request description:

  ### Description

  Updates to rust-bitcoin 0.30.0 and miniscript 0.10.0

  Not covered in this PR:
  - #1036. Although the latter is deprecated, I think it's better if I update it in a separate PR, as this one is pretty big already.
  - #1037
  - #1038

  Heads up, I'm explicitly denying multipath descriptors until we have better tests for them. See the commit message of 23fba7a

  ### Changelog notice

  - Update to `rust-bitcoin` 0.30.0 and `miniscript` 10.0.0

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

ACKs for top commit:
  evanlinjin:
    ACK 1da3b30

Tree-SHA512: ff1457ed711f9f8cdb446ea10aaf124632f539c02406da94317d8dc38013b321217d3bdcb2df4bd72b2ed92116b52e9c6b98ee91d4d508a579c67449a7caa549
@nondiremanuel nondiremanuel added this to the 1.0.0-alpha.3 milestone Aug 15, 2023
@nondiremanuel nondiremanuel moved this to Todo in BDK Aug 22, 2023
@realeinherjar
Copy link
Contributor

@notmandatory and @danielabrozzoni I want to try to help this one out.

@nondiremanuel nondiremanuel moved this from Todo to In Progress in BDK Sep 12, 2023
@notmandatory notmandatory modified the milestones: 1.0.0-alpha.3, 1.0.0-alpha.4 Nov 13, 2023
@nondiremanuel nondiremanuel modified the milestones: 1.0.0-alpha.4, 1.0.0 Jan 6, 2024
@evanlinjin
Copy link
Member

We need to discuss whether this needs to go into the BDK v1.0.0 release.

If we are overshooting while using .max_weight_to_satisfy, I think it is okay to leave it as is. It will be a large change to use the new rust-bitcoin methods.

If there is a chance for undershooting, that will be high risk and we should switch to use the new API.

@storopoli
Copy link
Contributor

@nondiremanuel assign me and unassign @realeinherjar.

@notmandatory notmandatory added module-wallet api A breaking API change labels Mar 18, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in BDK Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api A breaking API change module-wallet
Projects
Archived in project
6 participants