multi: recognize new standard P2A output type #2433
multi: recognize new standard P2A output type #2433Roasbeef wants to merge 8 commits intobtcsuite:masterfrom
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.
|
|
||
| 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.
|
@kcalvinalvin PTAL |
|
Most things look good. But Things done:
|
| // of the relay fee. This optimization avoids the more complex | ||
| // calculation below. | ||
| if txscript.IsPayToAnchorScript(txOut.PkScript) { | ||
| return txOut.Value < 240 |
There was a problem hiding this comment.
This hard-codes P2A dust at 240 sats regardless of minRelayTxFee. That matches the default P2A dust threshold because the existing witness dust formula already evaluates to 240 sats at the default rate, but it changes configured relay policy: with a zero relay fee, 239-sat P2A is still dust, and with a high relay fee, 240-sat P2A is still non-dust. Bitcoin Core still derives the threshold from the supplied dust relay feerate, so I think this should use the existing fee-sensitive dust calculation rather than special-casing IsDust.
There was a problem hiding this comment.
There was a problem hiding this comment.
Then what if people decide to increase/decrease it? Also just realized in btcd we are missing the config -dustrelayfee, similar to what bitcoind has https://github.com/bitcoin/bitcoin/blob/859215218667ca9f35d5adae0289e4a125798087/src/policy/policy.h#L63-L68
| // of the relay fee. This optimization avoids the more complex | ||
| // calculation below. | ||
| if txscript.IsPayToAnchorScript(txOut.PkScript) { | ||
| return txOut.Value < 240 |
There was a problem hiding this comment.
Then what if people decide to increase/decrease it? Also just realized in btcd we are missing the config -dustrelayfee, similar to what bitcoind has https://github.com/bitcoin/bitcoin/blob/859215218667ca9f35d5adae0289e4a125798087/src/policy/policy.h#L63-L68
| witnessProg, []byte{0x4e, 0x73}, | ||
| ) { | ||
| return &AddressPayToAnchor{ | ||
| hrp: hrp, |
There was a problem hiding this comment.
This stores the HRP directly from the input prefix, unlike the other segwit address constructors that normalize decoded HRPs to lowercase. A valid all-uppercase P2A address such as BC1PFEESSRAWGF will decode, and EncodeAddress() will re-emit lowercase, but IsForNet(&chaincfg.MainNetParams) returns false because the stored HRP is BC instead of bc.
Can we normalize the HRP here, or use the lowercase HRP returned by bech32 decoding, and add uppercase decode coverage?
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.
6 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.