-
-
Notifications
You must be signed in to change notification settings - Fork 9
Enforce nLockTime/sequence transaction finality at submit #181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
sirdeggen
wants to merge
5
commits into
main
Choose a base branch
from
claude-feat/locktime
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
db08193
feat: enforce nLockTime/sequence transaction finality at submit
sirdeggen e23cebb
fix(ci): bound uint32 conversions and annotate intentional nil return
sirdeggen 1fd073f
Potential fix for pull request finding
galt-tr acdc971
Merge branch 'main' into claude-feat/locktime
galt-tr 6048ff0
Merge branch 'main' into claude-feat/locktime
mrz1836 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| package validator | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "math" | ||
| "time" | ||
|
|
||
| sdkTx "github.com/bsv-blockchain/go-sdk/transaction" | ||
| ) | ||
|
|
||
| // lockTimeThreshold is the boundary (teranode util.ValidLockTime) below which | ||
| // nLockTime is interpreted as a block height and at or above which it is | ||
| // interpreted as a Unix timestamp. | ||
| const lockTimeThreshold = 500_000_000 | ||
|
|
||
| // ErrTxNotFinal is returned when a transaction is non-final: its nLockTime has | ||
| // not yet been reached and at least one input has a non-final sequence number, | ||
| // so the transaction could still be replaced before its locktime elapses and | ||
| // is therefore ineligible for the next block. | ||
| var ErrTxNotFinal = errors.New("transaction is not final") | ||
|
|
||
| // ChainTip supplies the current best block height, used to evaluate | ||
| // height-based nLockTime finality. The store satisfies this interface. It is | ||
| // optional: when no ChainTip is wired the height-locked case cannot be | ||
| // evaluated and is accepted rather than rejected (graceful degradation), which | ||
| // matches arcade's intake stance of not owning a chaintracker. | ||
| type ChainTip interface { | ||
| GetActiveTipBlockHeight(ctx context.Context) (uint64, error) | ||
| } | ||
|
|
||
| // allInputsFinal reports whether every input has a final sequence number | ||
| // (0xffffffff), which makes the transaction final regardless of its nLockTime. | ||
| func allInputsFinal(tx *sdkTx.Transaction) bool { | ||
| for _, in := range tx.Inputs { | ||
| if in.SequenceNumber != sdkTx.MaxTxInSequenceNum { | ||
| return false | ||
| } | ||
| } | ||
| return true | ||
| } | ||
|
|
||
| // validLockTime mirrors teranode util.ValidLockTime: an nLockTime below | ||
| // lockTimeThreshold is satisfied when blockHeight >= lockTime; at or above the | ||
| // threshold it is a timestamp satisfied when blockTime >= lockTime. | ||
| // blockHeight and blockTime are the values of the block in which the | ||
| // transaction would be mined. Since BIP113 the time-based comparison is | ||
| // against the 11-block median-time-past, not the block timestamp itself. | ||
| func validLockTime(lockTime, blockHeight, blockTime uint32) bool { | ||
| if lockTime < lockTimeThreshold { | ||
| return blockHeight >= lockTime | ||
| } | ||
| return blockTime >= lockTime | ||
| } | ||
|
|
||
| // IsFinal mirrors teranode util.IsTransactionFinal (consensus rule TNJ-13): a | ||
| // transaction is final when either of the following holds: | ||
| // - the sequence number of every input is final (0xffffffff), or | ||
| // - nLockTime is zero, or below lockTimeThreshold and not greater than | ||
| // blockHeight, or at/above lockTimeThreshold and not greater than blockTime. | ||
| // | ||
| // blockHeight and blockTime are the values of the block in which the | ||
| // transaction would be mined. | ||
| func IsFinal(tx *sdkTx.Transaction, blockHeight, blockTime uint32) bool { | ||
| if allInputsFinal(tx) { | ||
| return true | ||
| } | ||
| if tx.LockTime == 0 { | ||
| return true | ||
| } | ||
| return validLockTime(tx.LockTime, blockHeight, blockTime) | ||
| } | ||
|
|
||
| // checkFinality enforces transaction finality at submit time, sourcing the | ||
| // chain height (for height-based locktimes) from the configured ChainTip and | ||
| // the wall clock (an upper bound on median-time-past) for time-based | ||
| // locktimes. Cases that need chain state we cannot obtain are accepted rather | ||
| // than rejected so a transient chain-tip read failure never blocks a | ||
| // submission. Returns ErrTxNotFinal for a provably non-final transaction. | ||
| func (v *Validator) checkFinality(ctx context.Context, tx *sdkTx.Transaction) error { | ||
| // Decidable without chain state: all-inputs-final or nLockTime == 0. | ||
| if allInputsFinal(tx) || tx.LockTime == 0 { | ||
| return nil | ||
| } | ||
|
|
||
| if tx.LockTime < lockTimeThreshold { | ||
| // Height-based locktime: needs the current chain height. | ||
| if v.chainTip == nil { | ||
| return nil | ||
| } | ||
| h, err := v.chainTip.GetActiveTipBlockHeight(ctx) | ||
| if err != nil { | ||
| //nolint:nilerr // chain-tip read failure must not block submission (graceful degradation) | ||
| return nil | ||
| } | ||
| // A submitted tx targets the next block, so evaluate against height+1. | ||
| if validLockTime(tx.LockTime, clampToU32(h+1), 0) { | ||
| return nil | ||
| } | ||
| return ErrTxNotFinal | ||
| } | ||
|
|
||
| // Time-based locktime: wall clock is >= median-time-past, so a lock that | ||
| // has elapsed against now() has certainly elapsed against MTP. | ||
| now := v.now | ||
| if now == nil { | ||
| now = func() int64 { return time.Now().Unix() } | ||
| } | ||
| if validLockTime(tx.LockTime, 0, clampI64ToU32(now())) { | ||
| return nil | ||
| } | ||
| return ErrTxNotFinal | ||
| } | ||
|
|
||
| // clampToU32 saturates a uint64 to the uint32 range. Block heights never | ||
| // approach 2^32 in practice; the clamp makes the conversion provably bounded | ||
| // (gosec G115) instead of relying on that assumption. | ||
| func clampToU32(v uint64) uint32 { | ||
| if v > math.MaxUint32 { | ||
| return math.MaxUint32 | ||
| } | ||
| return uint32(v) | ||
| } | ||
|
|
||
| // clampI64ToU32 saturates an int64 (Unix seconds) to the uint32 range. Unix | ||
| // time stays within uint32 until 2106; negative values clamp to 0. | ||
| func clampI64ToU32(v int64) uint32 { | ||
| if v < 0 { | ||
| return 0 | ||
| } | ||
| if v > math.MaxUint32 { | ||
| return math.MaxUint32 | ||
| } | ||
| return uint32(v) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,153 @@ | ||
| package validator | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "testing" | ||
|
|
||
| sdkTx "github.com/bsv-blockchain/go-sdk/transaction" | ||
| ) | ||
|
|
||
| // finalInput returns an input whose sequence number is final (0xffffffff). | ||
| func finalInput() *sdkTx.TransactionInput { | ||
| return &sdkTx.TransactionInput{SourceTXID: nonZeroSourceTXID(), SequenceNumber: sdkTx.MaxTxInSequenceNum} | ||
| } | ||
|
|
||
| // nonFinalInput returns an input whose sequence number is NOT final, leaving | ||
| // the transaction subject to its nLockTime. | ||
| func nonFinalInput() *sdkTx.TransactionInput { | ||
| return &sdkTx.TransactionInput{SourceTXID: nonZeroSourceTXID(), SequenceNumber: 0} | ||
| } | ||
|
|
||
| func TestIsFinal_LockTimeZero(t *testing.T) { | ||
| // LockTime 0 is final regardless of sequence numbers. | ||
| tx := &sdkTx.Transaction{LockTime: 0, Inputs: []*sdkTx.TransactionInput{nonFinalInput()}} | ||
| if !IsFinal(tx, 0, 0) { | ||
| t.Error("locktime 0 must be final") | ||
| } | ||
| } | ||
|
|
||
| func TestIsFinal_AllInputsFinal(t *testing.T) { | ||
| // A future locktime is disabled when every input sequence is final. | ||
| tx := &sdkTx.Transaction{LockTime: 1_000_000_000, Inputs: []*sdkTx.TransactionInput{finalInput(), finalInput()}} | ||
| if !IsFinal(tx, 0, 0) { | ||
| t.Error("all-inputs-final must be final despite future locktime") | ||
| } | ||
| } | ||
|
|
||
| func TestIsFinal_HeightLockPassed(t *testing.T) { | ||
| // Height-based locktime (< 500000000) that is below the chain height | ||
| // has expired -> final, even with a non-final input. | ||
| tx := &sdkTx.Transaction{LockTime: 800_000, Inputs: []*sdkTx.TransactionInput{nonFinalInput()}} | ||
| if !IsFinal(tx, 800_001, 0) { | ||
| t.Error("height lock in the past must be final") | ||
| } | ||
| } | ||
|
|
||
| func TestIsFinal_HeightLockNotPassed(t *testing.T) { | ||
| tx := &sdkTx.Transaction{LockTime: 800_000, Inputs: []*sdkTx.TransactionInput{nonFinalInput()}} | ||
| if IsFinal(tx, 799_999, 0) { | ||
| t.Error("future height lock with non-final input must be non-final") | ||
| } | ||
| } | ||
|
|
||
| func TestIsFinal_TimeLockPassed(t *testing.T) { | ||
| // Time-based locktime (>= 500000000) not greater than blockTime has elapsed. | ||
| tx := &sdkTx.Transaction{LockTime: 1_700_000_000, Inputs: []*sdkTx.TransactionInput{nonFinalInput()}} | ||
| if !IsFinal(tx, 0, uint32(1_700_000_001)) { | ||
| t.Error("time lock in the past must be final") | ||
| } | ||
| } | ||
|
|
||
| func TestIsFinal_TimeLockNotPassed(t *testing.T) { | ||
| tx := &sdkTx.Transaction{LockTime: 1_700_000_000, Inputs: []*sdkTx.TransactionInput{nonFinalInput()}} | ||
| if IsFinal(tx, 0, uint32(1_699_999_999)) { | ||
| t.Error("future time lock with non-final input must be non-final") | ||
| } | ||
| } | ||
|
|
||
| func TestIsFinal_TimeLockNotPassedButOneInputFinalEnough(t *testing.T) { | ||
| // Mixed sequences: any single non-final input keeps the tx non-final. | ||
| tx := &sdkTx.Transaction{LockTime: 1_700_000_000, Inputs: []*sdkTx.TransactionInput{finalInput(), nonFinalInput()}} | ||
| if IsFinal(tx, 0, uint32(1_699_999_999)) { | ||
| t.Error("a single non-final input must keep an unexpired tx non-final") | ||
| } | ||
| } | ||
|
|
||
| func TestIsFinal_ThresholdMinusOne_UsesHeightNotTimestamp(t *testing.T) { | ||
| // lockTime == lockTimeThreshold-1 is still height-based. With blockHeight | ||
| // meeting it and blockTime zero, it must be final — proving the height | ||
| // branch was taken (a timestamp comparison against 0 would reject). | ||
| tx := &sdkTx.Transaction{LockTime: 499_999_999, Inputs: []*sdkTx.TransactionInput{nonFinalInput()}} | ||
| if !IsFinal(tx, 499_999_999, 0) { | ||
| t.Error("locktime 499999999 must be evaluated as a block height, not a timestamp") | ||
| } | ||
| } | ||
|
|
||
| func TestIsFinal_Threshold_UsesTimestampNotHeight(t *testing.T) { | ||
| // lockTime == lockTimeThreshold is the first timestamp-based value. With | ||
| // blockHeight large enough to satisfy a height comparison but blockTime | ||
| // zero, it must be non-final — proving the timestamp branch was taken (a | ||
| // height comparison would accept). | ||
| tx := &sdkTx.Transaction{LockTime: 500_000_000, Inputs: []*sdkTx.TransactionInput{nonFinalInput()}} | ||
| if IsFinal(tx, 500_000_000, 0) { | ||
| t.Error("locktime 500000000 must be evaluated as a timestamp, not a block height") | ||
| } | ||
| } | ||
|
|
||
| // fakeChainTip implements ChainTip for finality tests. | ||
| type fakeChainTip struct { | ||
| height uint64 | ||
| err error | ||
| } | ||
|
|
||
| func (f fakeChainTip) GetActiveTipBlockHeight(_ context.Context) (uint64, error) { | ||
| return f.height, f.err | ||
| } | ||
|
|
||
| func TestCheckFinality_NilChainTipSkipsHeightLock(t *testing.T) { | ||
| // Without a chain-tip source the height-based case cannot be evaluated; | ||
| // the validator must not reject it (graceful degradation). | ||
| v := NewValidator(nil) | ||
| tx := &sdkTx.Transaction{LockTime: 800_000, Inputs: []*sdkTx.TransactionInput{nonFinalInput()}} | ||
| if err := v.checkFinality(context.Background(), tx); err != nil { | ||
| t.Errorf("nil chain tip must accept height-locked tx, got %v", err) | ||
| } | ||
| } | ||
|
|
||
| func TestCheckFinality_RejectsFutureHeightLock(t *testing.T) { | ||
| v := NewValidator(nil) | ||
| v.SetChainTip(fakeChainTip{height: 799_998}) // next block = 799_999, < 800_000 | ||
| tx := &sdkTx.Transaction{LockTime: 800_000, Inputs: []*sdkTx.TransactionInput{nonFinalInput()}} | ||
| if err := v.checkFinality(context.Background(), tx); !errors.Is(err, ErrTxNotFinal) { | ||
| t.Errorf("expected ErrTxNotFinal, got %v", err) | ||
| } | ||
| } | ||
|
|
||
| func TestCheckFinality_AcceptsPassedHeightLock(t *testing.T) { | ||
| v := NewValidator(nil) | ||
| v.SetChainTip(fakeChainTip{height: 800_000}) // next block = 800_001, > 800_000 | ||
| tx := &sdkTx.Transaction{LockTime: 800_000, Inputs: []*sdkTx.TransactionInput{nonFinalInput()}} | ||
| if err := v.checkFinality(context.Background(), tx); err != nil { | ||
| t.Errorf("expected acceptance, got %v", err) | ||
| } | ||
| } | ||
|
|
||
| func TestCheckFinality_RejectsFutureTimeLock(t *testing.T) { | ||
| v := NewValidator(nil) | ||
| v.SetNowFunc(func() int64 { return 1_699_999_999 }) | ||
| tx := &sdkTx.Transaction{LockTime: 1_700_000_000, Inputs: []*sdkTx.TransactionInput{nonFinalInput()}} | ||
| if err := v.checkFinality(context.Background(), tx); !errors.Is(err, ErrTxNotFinal) { | ||
| t.Errorf("expected ErrTxNotFinal, got %v", err) | ||
| } | ||
| } | ||
|
|
||
| func TestCheckFinality_ChainTipErrorAccepts(t *testing.T) { | ||
| // A chain-tip read failure must not block submission. | ||
| v := NewValidator(nil) | ||
| v.SetChainTip(fakeChainTip{err: errors.New("boom")}) | ||
| tx := &sdkTx.Transaction{LockTime: 800_000, Inputs: []*sdkTx.TransactionInput{nonFinalInput()}} | ||
| if err := v.checkFinality(context.Background(), tx); err != nil { | ||
| t.Errorf("chain-tip error must accept, got %v", err) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sirdeggen thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this definitely needs to be fixed, the MTP can be calculated with header data only I believe that's possible via chaintracks.