bip54: add coinbase locktime, sequence (and version) to GBT

[Sjors]

BIP54 proposes constraining the coinbase transaction nLockTime and nSequence fields.

Bitcoin Core's internal mining code has been doing this since v30.0 (see bitcoin/bitcoin#32155), but currently the fields are only communicated to IPC clients (i.e. Stratum v2, see e.g. bitcoin/bitcoin#33819).

This PR extends BIP54 with the following BIP22 fields (getblocktemplate RPC):

• lock_time
• sequence
• version (not used by BIP54, but it makes getblocktemplate now cover all coinbase transaction fields)

Setting these fields makes miners forward compatible with BIP54, if it's ever activated, but is not the same as version bit signaling.

Reference implementation: bitcoin/bitcoin#34419

Mailinglist post: https://groups.google.com/g/bitcoindev/c/znBz5MA7_Bo/m/CY2uMIenAgAJ

[darosior]

I sympathize with the motivation here, as i considered extending GBT with those fields a year ago, but eventually decided against. The reason is i don't think it solves a real problem. If a pool wants to be forward-compatible with BIP 54 it already has all necessary information (the block height). If a pool does not want to spend the time to make the small tweak to their mining pool software, this is not helping.

I also don't think it hurts, and i can hear the argument that if these fields are given consensus meaning, then GBT "should"( ™️ ) provide these fields. I just think it does not help with BIP 54 forward compatibility.

Regardless of the desirability of these additions, i think those should be specified as part of the GBT BIP, not BIP 54.

[darosior]

Meta-comment: Sjors already knows what i think as we've been discussing it for a couple weeks, but we are curious to hear from others.

[melvincarvalho]

BIP54’s consensus fixes are currently uncontroversial.

Bundling new getblocktemplate additions into it changes that and expands the policy surface. Those additions should be specified separately, where they belong.

[Sjors]

@darosior I tried to make it a separate BIP initially, and it just led to confusion: Sjors#1. I'm not opposed to making it a separate BIP, but it needs to be kept in sync with BIP54 and they have to refer to each other. So it just seems simpler to keep them in one place.

If a pool does not want to spend the time to make the small tweak to their mining pool software, this is not helping.

This seems a bit condescending. Pools may feel more comfortable simply using a few extra getblocktemplate fields than having to interpret the BIP themselves and implementing it, even if you offer them personal assistance.

@melvincarvalho do you prefer to discuss on the mailing list or here? Cross-posting isn't necessary. I already replied on the list for now.

[darosior]

I didn't mean to be condescending, thanks for pointing it out. What i meant is that they already have all the tools in hand, and i don't think providing the fields in the GBT response is materially lowering the bar to make their software compatible. In fact having to upgrade to a later Bitcoin Core version that serves these (fairly redundant) fields may be a higher bar than just using the block height information they already get in their existing setup.