Skip to content

max_satisfaction_weight is only deprecated for some descriptors #637

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

Closed
storopoli opened this issue Feb 15, 2024 · 2 comments · Fixed by #638
Closed

max_satisfaction_weight is only deprecated for some descriptors #637

storopoli opened this issue Feb 15, 2024 · 2 comments · Fixed by #638

Comments

@storopoli
Copy link
Contributor

Currently max_satisfaction_weight is deprecated only for:

but not for:

  • Pkh:
    /// Computes an upper bound on the weight of a satisfying witness to the
    /// transaction.
    ///
    /// Assumes all ec-signatures are 73 bytes, including push opcode and
    /// sighash suffix. Includes the weight of the VarInts encoding the
    /// scriptSig and witness stack length.
    pub fn max_satisfaction_weight(&self) -> usize { 4 * (1 + 73 + BareCtx::pk_len(&self.pk)) }
  • Wpkh:
    /// Computes an upper bound on the weight of a satisfying witness to the
    /// transaction.
    ///
    /// Assumes all ec-signatures are 73 bytes, including push opcode and
    /// sighash suffix. Includes the weight of the VarInts encoding the
    /// scriptSig and witness stack length.
    pub fn max_satisfaction_weight(&self) -> usize { 4 + 1 + 73 + Segwitv0::pk_len(&self.pk) }

Is this the intended behavior?
If not, I can easily (and gladly) open a PR to add deprecation notices to Pkh and Wpkh.

Cc @evanlinjin who originally deprecated these.
Related bitcoindevkit/bdk#1036

@apoelstra
Copy link
Member

I think

  • They should be deprecated across the board.
  • The deprecation notice should link to discussion in Another attempt at max_satisfaction_weight fixes #476 and explain that we redefined what bytes are counted by the method and that the new one will return a different value.
  • We should add a since=10.0.0 field to all the deprecations.

@storopoli
Copy link
Contributor Author

Ok, I'll open a PR later today. Thanks for the clarification :)

apoelstra added a commit that referenced this issue Feb 17, 2024
7fc7661 Deprecate across the board max_satisfaction_weight (Jose Storopoli)

Pull request description:

  Adds a `since=10.0.0` to all `max_satisfaction_weight` deprecations. Adds a note telling users to check #476 for more details.

  Closes #637.

ACKs for top commit:
  apoelstra:
    ACK 7fc7661
  tcharding:
    ACK 7fc7661

Tree-SHA512: 90a75bd44d5b0bec5044fc58186323f6c992e43958a912d9d36a1bda411ef6156076ac2125ee6dc8806a742b0aef046ae1f540911301972c8c2f95bb02ec8980
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants