min_ntime field name is misleading and description can be improved

[GitGab19]

While fixing a bug on SRI (stratum-mining/stratum#1325), I started to discuss with @Sjors about the min_ntime field, which has a misleading name.

Also its description inside NewMiningJob, NewExtendedMiningJob and SetCustomMiningJob messages should be improved.

Context: stratum-mining/stratum#1325 (comment)

[Sjors]

In NewMiningJob it's defined as:

Smallest nTime value available for hashing for the new mining job.

That corresponds to the BIP23 mintime value returned by the Bitcoin Core getblocktemplate.

But the accompanying text says:

If the min_ntime field is set, the client MUST start to mine on the new job immediately after receiving this message, and use the value for the initial nTime.

For that that you should use the curtime field as defined in BIP22.

Of course Stratum v2 doesn't use the getblocktemplate RPC.

The SetNewPrevHash message defines:

header_timestamp | U32       | The nTime field in the block header at which the client should start (usually current time). This is NOT the minimum valid nTime value.  

So this refers to the old curtime and makes it clear it's not mintime / min_time.

Indeed the Template Provider I implemented uses the equivalent of curtime for this field.

One issue imo is that the SRI software uses a confusing variable name, min_ntime for something that reflects curtime.

Another issue is that I'm confused how the other roles are supposed to know about mintime, e.g. for NewMiningJob, if the Template Provider doesn't tell them.

One option is to simply never bother with mintime. Always use the equivalent of curtime. Consensus allows for lower values, but in practice it's fine to just treat is as the minimum time. It removes the need to use your own clock. Meanwhile Bitcoin Core ensures this value accounts for the timewarp rule as well as for situations where MTP + 1 is in the future (rare on mainnet, common on testnet4).

[Sjors]

The spec for the Template Provider role's SetNewPrevHash message could be more opinionated about header_timestamp. Especially if someone implements a TP on top of an alternative node.

It SHOULD be at least the current clock time. It MUST account for consensus rules regarding the minimum timestamp, i.e. MTP + 1. It SHOULD take the timewarp rule proposed in BIP94 into account, so that future activation is safe.

[Fi3]

I think that this has something to do with sv2/sv1 compatibility. @jakubtrnka if I remember correctly you proposed something about that in the past, can you please double check? Ty