multi: recognize new standard P2A output type #2433
Conversation
Pull Request Test Coverage Report for Build 20117557279Details
💛 - Coveralls |
| return nil, fmt.Errorf("unable to parse address: %v", err) | ||
| } | ||
|
|
||
| // P2A scripts don't have addresses. |
There was a problem hiding this comment.
there is an address for p2a outputs bc1pfeessrawgf
https://mempool.space/address/bc1pfeessrawgf
https://github.com/bitcoin/bitcoin/blob/25212dfdb4cd7291392b6a94130f658c5bfa0a48/test/functional/rpc_validateaddress.py#L171
There was a problem hiding this comment.
Looks like a stray line, but ExtractPkScriptAddrs should handle that. Will add a test for this.
| nilAddrErrStr) | ||
| } | ||
| return payToWitnessTaprootScript(addr.ScriptAddress()) | ||
| case *btcutil.AddressPayToAnchor: |
There was a problem hiding this comment.
nit
btcutil.AddressPayToAnchor isn't introduced until commit 3f99cfe0a9b0ed9a33e0c11136127e8b6e2906ea.
So this commit doesn't build.
| }, | ||
| { | ||
| name: "invalid - wrong version", | ||
| script: []byte{OP_0, OP_DATA_2, 0x4e, 0x73}, |
There was a problem hiding this comment.
Just a question: is OP_1 a version? Curious as to the test's name
nvm figured it out
|
|
||
| go 1.23.2 | ||
|
|
||
| replace github.com/btcsuite/btcd/btcutil => ./btcutil |
There was a problem hiding this comment.
Are we ok with this or is this temporary?
There was a problem hiding this comment.
This is temp. It's needed as we ref the newly added addr type in txscript/standard.go.
#1825 will resolve this circualr dep module issue. After this is merged, I'll be ale to update the module dep, then remove this.
btcutil/p2a_address_test.go
Outdated
| otherNet = &chaincfg.TestNet3Params | ||
| } | ||
| if addr.IsForNet(otherNet) { | ||
| t.Errorf("IsForNet(other) = true, want false") |
There was a problem hiding this comment.
What's this testing for? Why do we want to test "othernet-ness"
There was a problem hiding this comment.
Just that IsForNet properly detects if an address isn't for the target network.
I agree it's a trivial test, tbh I did it just so the test coverage reported is higher.
btcutil/p2a_address_test.go
Outdated
| t.Errorf("IsForNet(other) = true, want false") | ||
| } | ||
|
|
||
| expectedScript := []byte{0x51, 0x02, 0x4e, 0x73} |
There was a problem hiding this comment.
I think it's better if I we declare something like the declaration in txscript/standard.go
var PayToAnchorScript = []byte{OP_1, OP_DATA_2, 0x4e, 0x73}
There was a problem hiding this comment.
Yeah the issue with that again is the module issue I brought up which will be addressed by #1825. This is defined, but the btcutil module wasn't updated (as this was only added in this PR). So I had to add yet another replace to amek things work.
btcutil/address.go
Outdated
| // script bytes: OP_1 <0x4e73>. | ||
| func (a *AddressPayToAnchor) ScriptAddress() []byte { | ||
| // Return the fixed P2A script bytes. | ||
| return []byte{0x51, 0x02, 0x4e, 0x73} |
There was a problem hiding this comment.
Yeah here too. Would be better if we declared some variable and referred to it
There was a problem hiding this comment.
Actually this won't work, as it causes an import cycle. We can't import txscript from btcutil. The other changed worked at it was in btcutil_test.
In this commit, we modify the execution logic, such that the P2A witness program doesn't appear as a unknown witness program version. For execution, we just make sure the script passes.
P2A is standard if the witness and the sigscript is empty. We make a smol refactor to be able to write a unit test for the input standardness.
We make sure it'll be accepted into the mempool, and can be spent without a witness.
|
@kcalvinalvin PTAL |
5 participants
In this PR, we add initial recognition of the new P2A output type. It actually overloads the existing Taproot witness version, but with a distinct witness program. We adopt the proposed standardness rules to make only spends without any witness data standard. Utilities to parse the script, make addresses, etc - have also been added.
Note that this doesn't include the logic to support "ephemeral dust", which enables zero fee and zero value outputs under certain conditions.
A replace dir is used as we modified
btcutilin this PR.