BIP68 introduced relative timelocks. They are relative to the block(s) in which this transaction’s parent transaction(s) were mined. The timelock can be expressed as block height or time difference.
Mastering Bitcoin by Andreas Antonopoulos has
helpful background
on relative timelocks.
Mempool policy does not allow transactions that can not be mined in the next block because they’re not BIP68-final.
LockPoints are used to represent the block or time when a relative lock is satisfied. Since reorgs can affect when (or if) a transaction’s input were confirmed, LockPoints can become invalid.
#23897 makes it easier to understand and reason about this code by separating the lockpoints calculation logic from the lockpoints validation logic.
CalculateLockPointsAtTip returns a std::optional<LockPoints>. Under which circumstance(s) will this be a std::nullopt? Do you see any alternative approaches?
What’s the point of having LockPoints::maxInputBlock when we already have LockPoints::height?
Consider the function CheckSequenceLocksAtTip. Prior to this PR, is LockPoints* lp an in, out, or in,out parameter? What about after this PR?
How does this PR change the behaviour of CheckSequenceLocksAtTip()?
Which, if any, observable behaviour change does this PR introduce?
<LarryRuane> There's a fair amount of code that moved, and there's a type of git diff that shows that (`--colorMoved` and `--colorMovedWs` i believe), that helps simplify the diff
<LarryRuane> stickies-v: For a given transaction, it's the per-input height of the block that contains the output that this input is attempting to spend
<LarryRuane> So if that prevheight (for a particular input) is a low number, then the difference between that height and the current height would be a large number (many blocks ago), then perhaps the timelock has been satisfied (depending on the value of the timelock, obviously)
<LarryRuane> each tx input has an `nSequence` field (4 bytes), and it's stored there, I think? that field is often 0xffffffff (no timelock), but otherwise, it is a timelock of some kind
<LarryRuane> theStack: yes that's right, it's actually stored in both places! in the output (or technically the locking script), and in nSequence .. I don't really understand why (there have to be both)
<stickies-v> nLocktime is to OP_CTLV what nSequence is to OP_CSV - we have script-based and transaction-based timelocks for both relative and absolute timelcks
<LarryRuane> stickies-v: "can a single transaction have different `nSequence` values?" -- yes, I think the timelocks for ALL inputs must be satisfied, or else the tx isn't "finalized" yet (able to accept to mempool and relayed)
<stickies-v> LarryRuane: glad you mention it, because it's a very common source of confusion. for relative timelocks, you don't NEED to use OP_CSV. you can just use nSequence without any kind of script level locking, if you only want the transaction to be spendable when the transactions parents have enough ancestors
<stickies-v> however, OP_CSV allows you to (relatively) timelock _outputs_ (without showing that to the world until they're spent) instead of transactions
<LarryRuane> I did have a question about different inputs (nSequence) having different timelocks ... let's say the timelock is satisfied for one input but not another ... could the sender then steal back that output (spend it back to himself)?
<stickies-v> but interestingly, the validation of OP_CSV relies on the nSequence fields: the transaction _spending_ and OP_CSV output needs to set its nSequence field (on that input) sufficiently high so it satisfies the OP_CSV script pending path
<stickies-v> and re multiple `nSequence` values: indeed, a transaction can have a different `nSequence` value for each of its inputs, and _all_of them need to be satisfied in order for the transaction to be final
<theStack> so IIUC now, you can either only use relative transaction-based timelocks with nSequence fields (bip 68), OR relative script-based timelocks using OP_CSV, but then the transaction-based timelocks also have to match (bip68+bip112)... but not bip112 alone
<LarryRuane> stickies-v: CalculateSequenceLocks() looks across inputs and prevheights, figures out the earliest (min) height and time (that the tip has to reach) so that this tx is finalized (timelocks satisfied), could be in the future or the past
<stickies-v> theStack: no you can combine them. Look at it this way: nSequence limits when the transaction itself becomes final, OP_CSV limits when the transaction *spending it* can become final
<LarryRuane> CalculateLockPointsAtTip() creates and populates the prevheights list, each entry is the height that the output was mined that this input wants to spend, Then it passes this to CalculateSequenceLocks() ... so CalculateLockPointsAtTip is a higher-level function that uses CalculateSequenceLocks
<stickies-v> LarryRuane: yes exactly, `CalculateLockPointsAtTip` is just a wrapper that first calculates the `prevheights`, and then after passing that to `CalculateSequenceLocks` also calculates the `max_input_height` (which we'll discuss in a bit)
<stickies-v> `CalculateLockPointsAtTip` returns a `std::optional<LockPoints>`. Under which circumstance(s) will this be a `std::nullopt`? Do you see any alternative approaches?
<LarryRuane> Yes I agree, this PR makes all this code more understandable, but it still seems pretty messy, as in, if you were writing all this from scratch today, it could be a lot simpler (but you never know until you try) ... but such a rewrite would be very hard to get merged (understandably so, easy to break things)
<LarryRuane> stickies-v: if I'm reading the code correctly, std::nullopt only if one of the transaction inputs refers to an output that can't be found in the coins database (UTXO set).
<LarryRuane> but I was a little surprised by this, because wouldn't this be checked elsewhere already (where timelocks aren't being used at all, every input must refer to a UTXO)
<LarryRuane> stickies-v: I think you can get different kind of utxo views? Like, include mempool or not, as you wish? I've always been somewhat confused about that
<stickies-v> yeah exactly, it just takes a `CCoinsView` - however, it seems that in all callsites it's passed with a view that contains both unconfirmed and confirmed UTXOs
<stickies-v> I think it definitely can fail if we're passing it the wrong `CCoinsView`. However, if we're not expecting it to happen, shouldn't we just assert (or `Assume`) that to happen instead of just logging an error and move on? These silent failures always worry me. And perhaps making the return type unnecessarily convoluted
<LarryRuane> thanks.. one thing I think keeps many of us awake at night is, what if some tx or block relay or whatever causes every node in the world to `assert` (just a few is okay, but all???)
<LarryRuane> If a reorg occurs, the block referred to by `maxInputBlock` may no longer be part of the best chain, whereas a block of any given height often will still exist.
<LarryRuane> I think the idea is, when we accept a tx into the mempool (it passes its timelock check), we also cache its `LockPoints`, so then later, if there's a reorg, we don't have to re-check the timelocks if the `LockPoints` is still valid (performance improvement only, i think?)
<stickies-v> "performance improvement only, i think?" yes exactly! when reorging, we only need to invalidate those `LockPoints` that have a `maxInputBlock` that is higher than how deep the reorg is
<stickies-v> because relative timelocks always refer to when a previous transaction was confirmed, and a reorg can change when a transaction was confirmed (or even make it unconfirmed)