Extend the CSV timeout limit with another 4 bits #961
Conversation
Somehow the CECKey impl is really broken if you want to set a raw private key. Core master already has a new impl so I just took that one and replaces the usages. This will probably be equivalent with what would be rebased in at a later stage.
So that we can more easily group features under a single flag. Alternatively we might prefer to have independent flags.
stevenroose
force-pushed
the
extend-csv
branch
from
a9d897d to
06e1238
Compare
| // block prior. | ||
| nMinTime = std::max(nMinTime, nCoinTime + (int64_t)((txin.nSequence & CTxIn::SEQUENCE_LOCKTIME_MASK) << CTxIn::SEQUENCE_LOCKTIME_GRANULARITY) - 1); | ||
| const uint32_t sequenceLocktimeMask = SequenceLocktimeMask(flags); | ||
| nMinTime = std::max(nMinTime, nCoinTime + (int64_t)((txin.nSequence & sequenceLocktimeMask) << CTxIn::SEQUENCE_LOCKTIME_GRANULARITY) - 1); |
There was a problem hiding this comment.
Worth noting that the SEQUENCE_LOCKTIME_GRANULARITY still corresponds to 512 seconds. Should we introduce another type of granularity in dynafed to be 64 seconds.
From bip 68:
For time-based relative lock-time, 512 second granularity was chosen because bitcoin blocks are generated every 600 seconds. So when using block-based or time-based, the same amount of time can be encoded with the available number of bits. Converting from a sequence number to seconds is performed by multiplying by 512 = 2^9, or equivalently shifting up by 9 bits.
Not many people use time-based timelocks, so maybe doing this might be unnecessary and we are okay with a granularity of 512sec.
There was a problem hiding this comment.
Yeah, I think we should leave it alone. Liquid blocks are every minute anyway (and deviate from that less than Bitcoin deviates from 10 minutes).
There was a problem hiding this comment.
We currently support CSV going only upto 2^20 blocks which is
>>> (2**20 * 1) / 60 / 24 / 365
1.9950076103500762
about ~2 years into the future. Without too much additional change in code. We can change the mask from 0x000fffff to 0x003fffff to increase it to 8 years with a slight change.
We can get even more by using other bits(bits 23 through 30) but flag 1 << 22 is defined for checking the lock time type and the logic might get messy, so probably is not worth it.
But IMO we should use 22 bits.
|
After discussion with @roconnor-blockstream and others I think we need to change how we're going forward on this:
Given this implementation complexity, and the fact that we can use time-based locks to get ~388 days worth of locktime (and Liquid signers have honest timestamps, unlike Bitcoin miners), I think we should punt on this and include it in Taproot. |
|
Out of scope for Taproot as well. I think we've found enough workarounds (absolute timelocks, time-based relative timelocks) that it's fine to just punt on this. Users who need extreme timelocks can use Simplicity. |
3 participants
Builds on #960.