Use improved getCoinbaseTx() if available #59
Conversation
Sjors
force-pushed
the
2025/11/coinbase
branch
from
b31cc6c to
a0d4a73
Compare
|
Rebased after #57 and bumped |
Sjors
force-pushed
the
2025/11/coinbase
branch
from
a0d4a73 to
e1f0117
Compare
|
Updated against the latest changes in bitcoin/bitcoin#33819 (e.g. renaming |
Sjors
force-pushed
the
2025/11/coinbase
branch
from
e1f0117 to
ece49d3
Compare
|
Empty push after #58 was merged so Github sees it's only one commit now. |
| for (const auto& output : coinbase.required_outputs) { | ||
| m_coinbase_tx_outputs.push_back(output); |
There was a problem hiding this comment.
In the new constructor the loop just pushes all required_outputs:
This trusts that getCoinbase() already filtered to OP_RETURN outputs. The fallback path does
the filtering in ExtractCoinbaseTemplate(). This is correct assuming Bitcoin Core's
getCoinbase() returns pre-filtered outputs - just worth double-checking in Bitcoin Core.
There was a problem hiding this comment.
This trusts that getCoinbase() already filtered to OP_RETURN outputs.
Indeed. I think it's better that Bitcoin Core handles this filtering. This means we no longer have to assume only OP_RETURN outputs are possible.
There was a problem hiding this comment.
I think we can say the PR is ready for merging.
There was a problem hiding this comment.
Thanks, I plan to do that after bitcoin/bitcoin#33819 lands.
Sjors
changed the title
Sjors
changed the title
This frees up the name getCoinbaseTx() for the next commit. Changing a function name does not impact IPC clients, as they only consider the function signature and sequence number.
Avoid the brittle process of constructing the template coinbase fields from Bitcoin Core's dummy coinbase transaction. This uses the new getCoinbaseTx() interface method. Fall back to the old approach if that method is not available.
Sjors
force-pushed
the
2025/11/coinbase
branch
from
ece49d3 to
3f68384
Compare
|
Updated to match the latest Bitcoin Core PR version. The interface changes in that latest update only rename methods and variables, which doesn't impact IPC, so you can still use ece49d3. |
2 participants
Take advantage of the improved
getCoinbaseTx()introduced in bitcoin/bitcoin#33819 which provides aCoinbaseTemplatestruct.This lets us avoid parsing Bitcoin Core's dummy transaction. For backwards compatibility when using the deprecated
getCoinbaseRawTx()theExtractCoinbaseTxTemplatestill does that. This can be dropped after a Bitcoin Core release ships with the new method.Builds on: