This PR is greatly simplified and is supposed to have one or more follow-up PRs to complete functionality.
In this PR:
signing using single key (any type)
verifying any signed message (auto-detects BIP-322 vs legacy)
Missing features/limitations:
proof of funds (i.e. additional inputs) support
multisig or other custom address type support (I need feedback on how to do this; I assume some psbt thing would be good?)
(RPC) format is restricted to SIMPLE mode now; it may eventually be LEGACY, SIMPLE, or FULL. (In some cases, it will use FULL format but this is not selectable yet.)
timelock support
DrahtBot added the label
Consensus
on Jan 13, 2022
DrahtBot added the label
GUI
on Jan 13, 2022
DrahtBot added the label
interfaces
on Jan 13, 2022
DrahtBot added the label
RPC/REST/ZMQ
on Jan 13, 2022
DrahtBot added the label
Utils/log/libs
on Jan 13, 2022
DrahtBot added the label
Wallet
on Jan 13, 2022
kallewoof force-pushed
on Jan 13, 2022
ghost
commented at 1:04 pm on January 13, 2022:
none
Although SIGHASH_DEFAULT implies SIGHASH_ALL, AFAIK using SIGHASH_ALL is still possible for users who wish to waste 1 byte. So I think you need m_require_sighash_all && nHashType != SIGHASH_ALL here too.
kallewoof
commented at 10:18 pm on January 13, 2022:
Isn’t that a malleability? Anyway, gotcha.
in
src/rpc/misc.cpp:352
in
aed56aa486outdated
347@@ -348,9 +348,16 @@ static RPCHelpMan verifymessage()
348 throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to key");
349 case MessageVerificationResult::ERR_MALFORMED_SIGNATURE:
350 throw JSONRPCError(RPC_TYPE_ERROR, "Malformed base64 encoding");
351+ case MessageVerificationResult::ERR_POF:
352+ throw JSONRPCError(RPC_TYPE_ERROR, "Proof of funds require UTXO set access to verify"); // TODO: get access to UTXO set / mempool to handle this, then remove this error code?
This message implies the user can do something about it. Suggest instead: “BIP-322 Proof of Funds is not supported”
kallewoof
commented at 10:21 pm on January 13, 2022:
Sounds good. The intention is to actually add this functionality, but I think it should be its own PR.
in
src/rpc/misc.cpp:354
in
aed56aa486outdated
347@@ -348,9 +348,16 @@ static RPCHelpMan verifymessage()
348 throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to key");
349 case MessageVerificationResult::ERR_MALFORMED_SIGNATURE:
350 throw JSONRPCError(RPC_TYPE_ERROR, "Malformed base64 encoding");
351+ case MessageVerificationResult::ERR_POF:
352+ throw JSONRPCError(RPC_TYPE_ERROR, "Proof of funds require UTXO set access to verify"); // TODO: get access to UTXO set / mempool to handle this, then remove this error code?
353+ case MessageVerificationResult::INCONCLUSIVE:
354+ return false; // TODO: switch to a string based result? mix bool and strings?
kallewoof
commented at 10:23 pm on January 13, 2022:
Because these aren’t errors? But yeah, that would be one way to return a message. I think I’d prefer to expand the returned value from simple bool to true|false|“inconclusive”, but not sure that would be appreciated.
Some RPCs return false as the result and an error message field. Others just throw with an error code and message. It’s an error in the vague sense that the RPC failed to verify what it was asked to verify.
kallewoof
commented at 11:09 pm on January 14, 2022:
OK, but what about “OK_TIMELOCKED”?
in
src/util/message.h:54
in
aed56aa486outdated
35@@ -36,7 +36,23 @@ enum class MessageVerificationResult {
36 ERR_NOT_SIGNED,
3738 //! The message verification was successful.
39- OK
40+ OK,
41+
42+ //
43+ // BIP-322 extensions
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Conflicts
Reviewers, this pull request conflicts with the following ones:
#26008 (wallet: cache IsMine scriptPubKeys to improve performance of descriptor wallets by achow101)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
benthecarman
commented at 6:38 pm on January 13, 2022:
contributor
@kallewoof do you know if any other wallet implemented BIP-322 yet, so we can compare the implementation?
This is really a good time to add test vectors to the BIP (and this PR).
kallewoof
commented at 11:10 pm on January 13, 2022:
contributor
I will make test vectors very soon. Let’s compare notes at that point @benthecarman.
benthecarman
commented at 11:14 pm on January 13, 2022:
contributor
I will make test vectors very soon. Let’s compare notes at that point @benthecarman.
Sounds good, I made some in pr if you want to try those
kallewoof
commented at 11:15 pm on January 13, 2022:
contributor
Yep. I will try them, once I make my own. I don’t want to adjust the code to fit your test vectors, I’d rather both implementations independently passing both sets.
kallewoof force-pushed
on Jan 14, 2022
kallewoof force-pushed
on Jan 14, 2022
luke-jr
commented at 10:06 pm on January 15, 2022:
member
Would like to see BIP 322 address the distinction between the current “signer receives fund with the address” and the long-desired “signer sent a prior transaction” even if not supported by this PR.
kallewoof
commented at 12:10 pm on January 16, 2022:
contributor
@luke-jr I’m not sure if you’re suggesting a BIP change or a code feature. If the latter, I think the ideal path forward is for you to open a pull request follow-up to this one (maybe post-merge) to do what you are requesting. If former, open a PR to the BIP?
theStack
commented at 6:45 pm on January 19, 2022:
contributor
Strong Concept ACK
Thanks for working on this!
kallewoof
commented at 10:20 am on January 26, 2022:
contributor
theStack
commented at 1:24 pm on January 30, 2022:
Note that HASHER_BIP322 and some other identifiers are not available yet at this commit, i.e. compiling b23186d9f816aefe2fcf95e6f77d941c65e9c71c currently fails:
0... 1 CXX wallet/libbitcoin_wallet_tool_a-wallettool.o
2util/message.cpp:105:41: error: use of undeclared identifier 'HASHER_BIP322' 3 uint256 message_hash = (CHashWriter(HASHER_BIP322) << message).GetSHA256();
4^ 5util/message.cpp:116:22: error: use of undeclared identifier 'DecodeTx' 6if (signature && DecodeTx(to_sign, *signature, /* try_no_witness */ true, /* try_witness */ true)) {
7^ 8util/message.cpp:120:49: error: no member named 'ERR_POF'in'MessageVerificationResult' 9 result = MessageVerificationResult::ERR_POF;
10~~~~~~~~~~~~~~~~~~~~~~~~~~~^11util/message.cpp:128:49: error: no member named 'ERR_INVALID'in'MessageVerificationResult'12 result = MessageVerificationResult::ERR_INVALID;
13~~~~~~~~~~~~~~~~~~~~~~~~~~~^14util/message.cpp:141:57: error: no member named 'ERR_INVALID'in'MessageVerificationResult'15 result = MessageVerificationResult::ERR_INVALID;
16~~~~~~~~~~~~~~~~~~~~~~~~~~~^175 errors generated.18...
theStack
commented at 1:27 pm on January 30, 2022:
contributor
PR description nit:
(RPC) format is restricted to SINGLE mode now; it may eventually be LEGACY, SINGLE, OR FULL
This should likely be SIMPLE rather than SINGLE?
kallewoof force-pushed
on Jan 31, 2022
kallewoof force-pushed
on Jan 31, 2022
kallewoof
commented at 4:23 am on January 31, 2022:
contributor
@theStack Thanks for the review. You’re right, I meant SIMPLE not SINGLE. Fixed.
I did some clean-up and I think this also fixes the issues with HASHER_BIP322 being used before it is added.
kallewoof force-pushed
on Jan 31, 2022
kallewoof force-pushed
on Jan 31, 2022
kallewoof force-pushed
on Jan 31, 2022
kallewoof force-pushed
on Jan 31, 2022
kallewoof
commented at 10:11 am on January 31, 2022:
contributor
No idea why fuzzer runs out of time (2h!). Restarting didn’t fix it either.
maflcko
commented at 10:21 am on January 31, 2022:
member
This was fixed in 9237bdaac196951a437accaefa65638149b25978. Restarting the fuzz now should hopefully fix it.
kallewoof force-pushed
on Jan 31, 2022
kallewoof force-pushed
on Jan 31, 2022
kallewoof force-pushed
on Feb 1, 2022
kallewoof force-pushed
on Feb 2, 2022
kallewoof
commented at 3:44 am on February 2, 2022:
contributor
wip-abramson
commented at 4:03 pm on May 13, 2022:
01 // BIP322 signature created using buidl-python library with same parameters as test on line 2596
2 BOOST_CHECK_EQUAL(
3 MessageVerify(
4 "bc1q9vza2e8x573nczrlzms0wvx3gsqjx7vavgkx0l",
5 "AkgwRQIhAOzyynlqt93lOKJr+wmmxIens//zPzl9tqIOua93wO6MAiBi5n5EyAcPScOjf1lAqIUIQtr3zKNeavYabHyR8eGhowEhAsfxIAMZZEKUPYWI4BruhAQjzFT8FSFSajuFwrDL1Yhy",
6 "Hello World"),
7 MessageVerificationResult::OK);
8
We tried to create an implementation of BIP322 using the buidl-python library. The signature in this test case was produced using the same message and address. Not sure what is going on here.
I was also able to verify the signature provided in the test on line 2596 through the buidl python code I produced.
So two signatures for the same message for the same address are both returning as valid. This seems strange?
@wip-abramson ECDSA signatures have randomness in them, so it is perfectly possible to construct multiple valid ones for the same key and message. Bitcoin Core’s signing logic is based on RFC6979, but additionally grinds the nonce in order to make sure the R and S values in signatures don’t exceed 32 bytes each. Is your signing code doing the same?
@jandrieu It is an inevitability. No unique signature scheme based on the discrete logarithm problem that Bitcoin relies on is known (unique signatures schemes are ones where only a single signature is valid per (message, public key) pair).
kallewoof force-pushed
on May 16, 2022
DrahtBot removed the label
Needs rebase
on May 16, 2022
kallewoof force-pushed
on May 16, 2022
kallewoof force-pushed
on May 17, 2022
wip-abramson
commented at 12:59 pm on May 17, 2022:
none
Whilst implementing bip322 I noticed a discrepancy in the test vectors in the actual BIP vs the ones used in this P.R. The ones currently in bip322 are incorrect and don’t verify against the messages provided. The above-mentioned P.R. is a fix for this
laanwj removed the label
Consensus
on May 26, 2022
laanwj removed the label
Utils/log/libs
on May 26, 2022
laanwj removed the label
interfaces
on May 26, 2022
in
src/test/util_tests.cpp:2720
in
64ecca3f0eoutdated
wip-abramson
commented at 6:00 pm on July 11, 2022:
This is a test case that demonstrates the issue with taproot signatures that @richerandprettier refers to. It seems that p2tr signatures validate against any message under the current impl.
Is there some extra configuration required to setup bitcoin with schnorr & taproot, wondering if these sigs are processed at all?
Thanks for the test! Immensely helpful. Looking into what’s causing the issue now.
kallewoof
commented at 0:39 am on July 12, 2022:
contributor
Tested for Legacy, P2SH-SegWit, SegWit, and Taproot. (commit 64ecca3)
[..]
Except for Taproot addresses, which for some reason when the message is malformed, still results in true
Thanks a lot for testing! Looking into taproot issues.
kallewoof force-pushed
on Jul 12, 2022
kallewoof force-pushed
on Jul 12, 2022
kallewoof force-pushed
on Jul 12, 2022
luke-jr
commented at 3:10 am on July 12, 2022:
member
I’m not sure if you’re suggesting a BIP change or a code feature.
This is a BIP incompleteness thing that should be addressed before any implementation is considered for merging. The BIP gives a technical way to “sign a message”, without defining what the signature actually means.
Current signed messages only mean “the message is from whoever receives/d coins that are/were sent to this address”. Yet there’s relatively little correct use of it. Instead, people seem to want things like proof-of-funds and proof-of-sending, which are fundamentally different in nature (eg, not associated with an address). The BIP should define each of these use cases distinctly, so they can’t be misrepresented as the wrong kind of proof.
kallewoof
commented at 3:18 am on July 12, 2022:
contributor
The SCRIPT_VERIFY_TAPROOT flag was not added to the required flags, resulting in taproot checks being auto-successes. Fixed, thanks for the help!
Edit: thinking about this, this is not how BIP-322 is meant to behave when it encounters unknown opcodes and such; the “inconclusive” middle state is meant to address exactly this, and not just throw a “true” out blindly. Annoying.
Edit 2: this ends up being a silly special case for taproot, but it’s worth taking note of as we may have the same issues in future versions as we upgrade the BIP322_REQUIRED_FLAGS vs BIP322_INCONCLUSIVE_FLAGS. The inconclusive flags checks for upgraded taproot versions and such, which current taproot is not, whereas the required flags neglected to include the current taproot.
kallewoof
commented at 3:32 am on July 12, 2022:
contributor
This is a BIP incompleteness thing that should be addressed before any implementation is considered for merging. The BIP gives a technical way to “sign a message”, without defining what the signature actually means.
The motivation is quite clear on what it means, IMO:
Ultimately no message signing protocol can actually prove control of funds, both because a signature is obsolete as soon as it is created, and because the possessor of a secret key may be willing to sign messages on others’ behalf even if it would not sign actual transactions. No signmessage protocol can fix these limitations.
If you have suggestions on improvements to make, please make a pull request to the BIP repository.
Rspigler
commented at 6:03 am on July 12, 2022:
contributor
Would it not be clearer to have the verifymessage command take the arguments this way:
verifymessage "address" "message" "signature" -> true/false
DrahtBot added the label
Needs rebase
on Jul 12, 2022
jandrieu
commented at 10:42 pm on July 12, 2022:
none
This is a BIP incompleteness thing that should be addressed before any implementation is considered for merging. The BIP gives a technical way to “sign a message”, without defining what the signature actually means.
This is not a problem specific to BIP-322. It’s generally handled at another layer. For example, in Linked Data Signatures, a ProofPurpose is used to clarify why that particular proof is generated. For Verifiable Credentials, that proof purpose must be an attestationMethod, which means the signature was created as an attestation by the signer to the veracity of the signed document.
In contrast, zCaps, a capability mechanism that uses LinkedData signatures uses the two proof purposes of CapabilityInvocation and CapabilityDelegation to indicate the meaning of different signatures. The former to indicate that the signature means to “do” the action that is invoked, the latter that the signature means the previous controller is delegating to someone else (and not, at this time, invoking anything).
In all cases, the proof that is verified is cryptographic, and BIP-322 can provide that verifiable proof, while the proof-purpose is part of the signed payload yet exogenous to the signing itself.
FWIW, @wip-abramson and I are working on mapping BIP-322 to Verifiable Credentials. I’m not sure if he’s shared that work with this conversation, but it is a good example of how you can generalize BIP-322 signatures for specific use cases.
luke-jr
commented at 11:06 pm on July 12, 2022:
member
This is not a problem specific to BIP-322. It’s generally handled at another layer.
It can’t be. It’s a different person signing in each scenario.
Proof-of-funds can be anyone who could spend the UTXOs. Proof-of-sender can only be the actual person who did send a given transaction. Proof-of-address can only be the person who received or would receive funds sent to an address. These can all be distinct entities, so the signature itself must specify which it is testifying to.
The existing signmessage standard/RPC is only capable of proof-of-address, and cannot prove funds or senders.
jandrieu
commented at 3:24 pm on July 13, 2022:
none
This is not a problem specific to BIP-322. It’s generally handled at another layer.
It can’t be. It’s a different person signing in each scenario.
I’m not sure what you mean, but that would seem to support my point that a signature itself does not describe why the signature was made.
Proof-of-funds can be anyone who could spend the UTXOs. Proof-of-sender can only be the actual person who did send a given transaction. Proof-of-address can only be the person who received or would receive funds sent to an address. These can all be distinct entities, so the signature itself must specify which it is testifying to.
The existing signmessage standard/RPC is only capable of proof-of-address, and cannot prove funds or senders.
On that, I agree. You can apply BIP-322 to get a contributing factor for proof-of-funds, but BIP-322 itself does not establish that.
All it can establish is that the given cryptographic puzzle of a specific script is satisfied with respect to a given message.
Whether or not you treat that valid signature as proof-of-funds or not is at a different layer. At a minimum, you would need to apply the logic that each of the gathered UTXOs is, in fact, still unspent. You will also need to decide if the solved puzzle should be interpreted as proof of funds for your use case, given that BIP-322 does not lock the funds.
Would it not be clearer to have the verifymessage command take the arguments this way: verifymessage "address" "message" "signature" -> true/false
It would, but unfortunately this order was set long before this pull request was created, and who knows what systems out there are depending on the order being what it is today. It may be worth it to do some simple analysis on the arguments, and swap them automatically based on which of “message” and “signature” looks like a signature, but I don’t know if that would be appreciated, as it’s quite an unusual thing to do, at least in this code base.
kallewoof force-pushed
on Jul 14, 2022
kallewoof force-pushed
on Jul 14, 2022
kallewoof force-pushed
on Jul 14, 2022
DrahtBot removed the label
Needs rebase
on Jul 14, 2022
DrahtBot added the label
Needs rebase
on Jul 22, 2022
in
src/util/message.cpp:89
in
85ed08c22coutdated
89+ }
90+
91+ case MessageSignatureFormat::SIMPLE:
92+ case MessageSignatureFormat::FULL:
93+ {
94+ CHashWriter hasher(HASHER_BIP322);
134+ CMutableTransaction to_sign;
135+ if (signature && DecodeTx(to_sign, *signature, /* try_no_witness */ true, /* try_witness */ true)) {
136+ // validate decoded transaction
137+ // multiple inputs (proof of funds) are not supported as we do not have UTXO set access
138+ if (to_sign.vin.size() > 1) {
139+ result = MessageVerificationResult::ERR_POF;
in
src/rpc/signmessage.cpp:55
in
30642a1859outdated
48@@ -49,9 +49,16 @@ static RPCHelpMan verifymessage()
49 throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to key");
50 case MessageVerificationResult::ERR_MALFORMED_SIGNATURE:
51 throw JSONRPCError(RPC_TYPE_ERROR, "Malformed base64 encoding");
52+ case MessageVerificationResult::ERR_POF:
53+ throw JSONRPCError(RPC_TYPE_ERROR, "BIP-322 Proof of funds is not yet supported"); // TODO: get access to UTXO set / mempool to handle this, then remove this error code?
54+ case MessageVerificationResult::INCONCLUSIVE:
55+ return false; // TODO: switch to a string based result? mix bool and strings?
In 5bc21371947340a35039cb9ce95571b45bc535a4 “message: add BIP322Txs class for preparing BIP-322 transactions”
Since both BIP 322 and BIP 325 have very similar transaction construction functions, what do you think about de-deduplicating and having SignetTxs::Create call this function? It would require a minor tweak to this function to take a vector of bytes to push onto to_spend’s input’s scriptSig.
That is the plan yeah, but I was considering it to be the follow-up next step after this PR is merged, to keep this minimal. As you say the follow-up PR to fix this should be quite trivial and easy to review.
in
src/script/interpreter.h:303
in
62f307f442outdated
If it is merged with the BIP-325 variant, it does need to be.
in
src/qt/signverifymessagedialog.cpp:228
in
8f9c561c7boutdated
223+ );
224+ return;
225+ case MessageVerificationResult::ERR_POF:
226+ // TODO: support proof of funds verifications
227+ ui->statusLabel_VM->setText(
228+ tr("Proof of funds verification unavailable right now.")
I would like to avoid using the same message for different errors, as it makes troubleshooting more difficult. That said, maybe “Some check failed” is not the right wording here…
In f0747683eb73f2052b9ef18b8246e81411c30e20 “wallet: add SignMessageBIP322() helper method to ScriptPubKeyMan”
This could be an assert. It should not be possible to reach this point without having a valid destination, and as we do not provide a signature argument, Create should never fail.
It shouldn’t. First, both the RPC and the GUI will validate that address is a valid destination before passing it on. Then CWallet::SignMessage also ensures that there is a valid destination by getting the script for the destination and using the ScriptPubKeyMan that can provide for that script. So by the time we get to this line, we’ve already checked at least twice that the destination is valid.
In 30642a185971555ea68bd5b80f87d633ddd09593 “message: add BIP-322 verification support (without POF)”
This is unnecessary and will always fail.
By the time we reach this line, the destination must be PKHash and we must have been able to recover a pubkey from the signature, which means that the signature must have been 65 bytes, otherwise RecoverCompact will have failed. However, a BIP322 signature for PKHash must be greater than 65 bytes because it will include a full transaction for the p2pkh address. So if we do reach this line, the signature is just invalid and there’s no need to attempt a MessageVerifyBIP322.
I took the liberty of pasting your reasoning as a comment in case future reviewers wonder why a BIP322 attempt isn’t made. I can drop that if it seems overkill.
kallewoof force-pushed
on Jul 28, 2022
kallewoof force-pushed
on Jul 28, 2022
DrahtBot removed the label
Needs rebase
on Jul 28, 2022
luke-jr
commented at 6:56 am on July 28, 2022:
member
theStack
commented at 10:47 pm on September 20, 2022:
nit: DecodeBase64 takes a std::string_view, i.e. passing a string directly is fine:
0 auto signature_bytes = DecodeBase64(signature);
in
src/util/message.cpp:118
in
5f651713e9outdated
121- return MessageVerificationResult::ERR_PUBKEY_NOT_RECOVERED;
122+ return MessageVerifyBIP322(destination, *signature_bytes, message, MessageVerificationResult::ERR_PUBKEY_NOT_RECOVERED);
123 }
124125 if (!(CTxDestination(PKHash(pubkey)) == destination)) {
126+ // By the time we reach this line, the destination must be `PKHash` and we must have been able to recover a pubkey from the signature, which means that the signature must have been 65 bytes, otherwise `RecoverCompact` will have failed. However, a BIP322 signature for `PKHash` must be greater than 65 bytes because it will include a full transaction for the p2pkh address. So if we do reach this line, the signature is just invalid and there's no need to attempt a `MessageVerifyBIP322`.
theStack
commented at 10:51 pm on September 20, 2022:
With almost 500 characters, this comment is definitely too long for one line and should be split into multiple ones.
theStack
commented at 10:57 pm on September 20, 2022:
contributor
Left a bunch of code-review comments (mostly nits) below. I’m planning to do a second more in-depth review round within the next days.
aureleoules
commented at 2:22 pm on September 21, 2022:
3b00ed9a52d91864665f074eadaafc179344ba56
0 if (message_challenge.empty()) {
in
src/util/message.cpp:216
in
e54e5eef78outdated
203+ // multiple inputs (proof of funds) are not supported as we do not have UTXO set access
204+ if (to_sign.vin.size() > 1) {
205+ result = MessageVerificationResult::ERR_POF;
206+ return std::nullopt;
207+ }
208+ if ((to_sign.vin.size() == 0 || to_sign.vin[0].prevout.hash != to_spend.GetHash()) ||
aureleoules
commented at 2:23 pm on September 21, 2022:
3b00ed9a52d91864665f074eadaafc179344ba56
0 if ((to_sign.vin.empty() || to_sign.vin[0].prevout.hash != to_spend.GetHash()) ||
test: Add externally generated signature (buidl-python library) to verify tests
Co-authored-by: Will Abramson <wip.abramson@gmail.com>
63a2f592c3
Taproot tests
Co-authored-by: Will Abramson <wip.abramson@gmail.com>
de070b3a95
2-of-3 p2sh, 3-of-3 p2wsh
Co-authored-by: Will Abramson <wip.abramson@gmail.com>
29b28d07fa
kallewoof force-pushed
on Sep 26, 2022
in
src/wallet/scriptpubkeyman.cpp:234
in
29b28d07fa
229+ // TODO: this may be a multisig which successfully signed but needed additional signatures
230+ return SigningResult::SIGNING_FAILED;
231+ }
232+
233+ // We force the format to FULL, if this turned out to be a legacy format (p2pkh) signature
234+ if (to_sign.vin[0].scriptSig.size() > 0 || to_sign.vin[0].scriptWitness.IsNull()) {
aureleoules
commented at 10:56 am on November 7, 2022:
b32c238dd5bc413b4b03f99ae68f7e2b78b42d62
0 if (!to_sign.vin[0].scriptSig.empty() || to_sign.vin[0].scriptWitness.IsNull()) {
aureleoules approved
aureleoules
commented at 10:58 am on November 7, 2022:
contributor
I verified that signmessage/verifymessage works by manually creating signatures using every output type (legacy, p2sh-segwit, bech32, and bech32m).
I also verified that the scheme used in this PR matches the BIP322 specification.
Left some nits.
FWIW, here is the missing BIP322 tx-creation test for bech32m (taproot) using a key-path spend (though the internals don’t really matter here, since we don’t sign anyway), feel free to pick it up:
2713+ // BIP322 signature created using buidl-python library with same parameters as test on line 2596
2714+ BOOST_CHECK_EQUAL(
2715+ MessageVerify(
2716+ "bc1q9vza2e8x573nczrlzms0wvx3gsqjx7vavgkx0l",
2717+ "AkgwRQIhAOzyynlqt93lOKJr+wmmxIens//zPzl9tqIOua93wO6MAiBi5n5EyAcPScOjf1lAqIUIQtr3zKNeavYabHyR8eGhowEhAsfxIAMZZEKUPYWI4BruhAQjzFT8FSFSajuFwrDL1Yhy",
2718+ "Hello World"),
kyranjamie
commented at 9:27 am on March 25, 2023:
I get this value in my implementation, yet it is not reflected in the BIP
seannybird approved
seannybird approved
kallewoof closed this
on Aug 3, 2023
benma
commented at 10:08 am on October 26, 2023:
none
@kallewoof could you leave a comment explaining why you closed this PR? It is not apparent from looking at the history of this PR what the state is of BIP-322 support in Bitcoin Core.
luke-jr referenced this in commit
bea146a3d4
on Mar 12, 2024
luke-jr referenced this in commit
793c1a06eb
on Mar 12, 2024
luke-jr referenced this in commit
34b76ac4ef
on Mar 12, 2024
luke-jr referenced this in commit
8e73fb5740
on Mar 12, 2024
luke-jr referenced this in commit
3406fe093f
on Mar 12, 2024
luke-jr referenced this in commit
50a364e878
on Mar 12, 2024
luke-jr referenced this in commit
2dff30e28c
on Mar 12, 2024
luke-jr referenced this in commit
458c5d414f
on Mar 12, 2024
luke-jr referenced this in commit
efeb6ab6a4
on Mar 12, 2024
luke-jr referenced this in commit
ba75a16dad
on Mar 12, 2024
luke-jr referenced this in commit
f2f4637b16
on Mar 12, 2024
luke-jr referenced this in commit
112eb4e5a9
on Mar 12, 2024
luke-jr referenced this in commit
9ab7b8ada6
on Mar 12, 2024
kilrau
commented at 8:52 pm on January 12, 2025:
none
Same question @kallewoof , why did you close this? It’s a much needed feature.
luke-jr referenced this in commit
47e3139dfe
on Feb 15, 2025
luke-jr referenced this in commit
a3ceaa2b45
on Feb 15, 2025
luke-jr referenced this in commit
24dd372849
on Feb 15, 2025
luke-jr referenced this in commit
db70c1af06
on Feb 15, 2025
luke-jr referenced this in commit
5a28c9078a
on Feb 15, 2025
luke-jr referenced this in commit
88129da337
on Feb 15, 2025
luke-jr referenced this in commit
bca3280898
on Feb 15, 2025
luke-jr referenced this in commit
99a971f479
on Feb 15, 2025
luke-jr referenced this in commit
ca4f8a6d20
on Feb 15, 2025
luke-jr referenced this in commit
6c52eb975e
on Feb 15, 2025
luke-jr referenced this in commit
05139da200
on Feb 15, 2025
luke-jr referenced this in commit
ec510b0298
on Feb 15, 2025
luke-jr referenced this in commit
954cc8c5a9
on Feb 15, 2025
luke-jr referenced this in commit
0f48cd9c7e
on Mar 5, 2025
luke-jr referenced this in commit
cae8713c1c
on Mar 5, 2025
luke-jr referenced this in commit
0bac67599a
on Mar 5, 2025
luke-jr referenced this in commit
b2a6f1fb47
on Mar 5, 2025
luke-jr referenced this in commit
9a2f15e9ba
on Mar 5, 2025
luke-jr referenced this in commit
217c7fe699
on Mar 5, 2025
luke-jr referenced this in commit
49b6b6d844
on Mar 5, 2025
luke-jr referenced this in commit
fd19826656
on Mar 5, 2025
luke-jr referenced this in commit
36708a812a
on Mar 5, 2025
luke-jr referenced this in commit
2ac8749197
on Mar 5, 2025
luke-jr referenced this in commit
be74da0a93
on Mar 5, 2025
luke-jr referenced this in commit
b34327b2c7
on Mar 5, 2025
luke-jr referenced this in commit
2ac412c746
on Mar 5, 2025
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2025-05-19 15:12 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me
