Introduce PsbtSigHashType #779
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We do not want to imports from within the lib and external of lib in the same line
sanket1729
force-pushed
the
psbt_ecdsa_sighash_ty
branch
from
9aad350
to
ebdeed0
Compare
dr-orlovsky
approved these changes
Jan 14, 2022
There was a problem hiding this comment.
ACK ebdeed0
Do not want to bikeshedd about method names, just left a note which can be applied only if there will be re-base or other updates
| } | ||
|
|
||
| /// Obtains the inner sighash byte from this [`PsbtSigHashType`]. | ||
| pub fn inner(self) -> u32 { |
There was a problem hiding this comment.
nit: probably as_u32 will be better, because it is not clear what is the inner type for PSBT sighashes. Also inner is quite uncommon, usually it is as_inner or into_inner.
Also as_u32 will match methods in EcdsaSigHashType
There was a problem hiding this comment.
Let's leave it for some of the follow-up PRs
|
Ok, I was the second reviewer, so I tested it on my machine with a test build script on each of the commits and merged. |
This was referenced Jan 14, 2022
Closed
sander2
added a commit
to sander2/polkabtc-clients
that referenced
this pull request
Oct 6, 2022
sander2
added a commit
to sander2/polkabtc-clients
that referenced
this pull request
Oct 6, 2022
sander2
added a commit
to sander2/polkabtc-clients
that referenced
this pull request
Oct 7, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We cannot really use
Psbtfor taproot because the sighash type is currently EcdsaSigHashType. We could introduce an enum with two options but then deser is not really clear, so I chose the approach in the current PR. Feedback or other ways to do this welcome :)This is NOT related to #776