diff --git a/pkg/maintainer/spv/spv.go b/pkg/maintainer/spv/spv.go index 20bd84bd3c..fef1a8c977 100644 --- a/pkg/maintainer/spv/spv.go +++ b/pkg/maintainer/spv/spv.go @@ -9,6 +9,7 @@ import ( "sync" "time" + "github.com/btcsuite/btcd/blockchain" "github.com/keep-network/keep-core/pkg/tbtc" "github.com/ipfs/go-log/v2" @@ -19,13 +20,8 @@ import ( var logger = log.Logger("keep-maintainer-spv") -// The length of the Bitcoin difficulty epoch in blocks. -const difficultyEpochLength = 2016 - -// The maximum number of block headers allowed in a single SPV proof. Bounds -// the forward walk over headers when computing required confirmations -// (relevant on testnet4 where long runs of minimum-difficulty blocks occur). -const maxProofHeaders = 144 +// minDifficultyTarget matches the Bridge's BTCUtils.DIFF1_TARGET. +var minDifficultyTarget = blockchain.CompactToBig(0x1d00ffff) func Initialize( ctx context.Context, @@ -375,13 +371,6 @@ func getProofInfo( headerCount := uint(0) for { - if headerCount >= maxProofHeaders { - // Could not find a decisive header or accumulate enough - // difficulty within a sane number of headers. Skip the - // transaction; it may become provable later. - return false, 0, 0, nil - } - blockHeight := proofStartBlock + uint64(headerCount) if blockHeight > uint64(latestBlockHeight) { // Not enough mined blocks yet to assemble the proof. Report the @@ -399,13 +388,22 @@ func getProofInfo( ) } + headerTarget := header.Target() + if headerTarget.Sign() <= 0 { + return false, 0, 0, fmt.Errorf( + "invalid target [%v] for block header at height [%v]", + headerTarget, + blockHeight, + ) + } + headerDiff := header.Difficulty() headerCount++ observedDiff.Add(observedDiff, headerDiff) if requestedDiff == nil { // Still looking for the decisive header. - if skipMinDifficulty && headerDiff.Cmp(one) == 0 { + if skipMinDifficulty && headerTarget.Cmp(minDifficultyTarget) == 0 { continue } diff --git a/pkg/maintainer/spv/spv_test.go b/pkg/maintainer/spv/spv_test.go index 088c619883..f97dbacb2e 100644 --- a/pkg/maintainer/spv/spv_test.go +++ b/pkg/maintainer/spv/spv_test.go @@ -27,7 +27,9 @@ func TestGetProofInfo(t *testing.T) { currentEpochDifficulty *big.Int previousEpochDifficulty *big.Int headerDifficultyAt func(uint) *big.Int + headerAt func(uint) *bitcoin.BlockHeader headersFrom, headersTo uint + expectedError string expectedIsProofWithinRelayRange bool expectedAccumulatedConfirmations uint expectedRequiredConfirmations uint @@ -147,20 +149,63 @@ func TestGetProofInfo(t *testing.T) { expectedAccumulatedConfirmations: 0, expectedRequiredConfirmations: 0, }, - // A run of minimum-difficulty headers longer than maxProofHeaders - // never reaches a decisive header. - "minimum difficulty run exceeds header bound": { - transactionConfirmations: 150, + // This header's target is harder than the exact DIFF1 target, but its + // integer difficulty rounds down to one. The Bridge does not skip it; + // it treats it as the decisive header and rejects it because it matches + // neither relay epoch difficulty. The maintainer must do the same. + "non-DIFF1 target rounding to difficulty one is not skipped": { + transactionConfirmations: 1, currentEpochDifficulty: diff(32), previousEpochDifficulty: diff(16), - headerDifficultyAt: func(uint) *big.Int { return diff(1) }, - headersFrom: proofStart, - headersTo: proofStart + 149, + headerAt: func(uint) *bitcoin.BlockHeader { + return &bitcoin.BlockHeader{Bits: 0x1d00aaaa} + }, + headersFrom: proofStart, + headersTo: proofStart, + + expectedIsProofWithinRelayRange: false, + expectedAccumulatedConfirmations: 0, + expectedRequiredConfirmations: 0, + }, + // Compact bits can decode to a zero target. Reject it before calling + // BlockHeader.Difficulty, which would otherwise divide by zero. + "zero target is rejected": { + transactionConfirmations: 1, + currentEpochDifficulty: diff(32), + previousEpochDifficulty: diff(16), + headerAt: func(uint) *bitcoin.BlockHeader { + return &bitcoin.BlockHeader{Bits: 0} + }, + headersFrom: proofStart, + headersTo: proofStart, + expectedError: "invalid target [0] for block header at height [790270]", expectedIsProofWithinRelayRange: false, expectedAccumulatedConfirmations: 0, expectedRequiredConfirmations: 0, }, + // Long testnet4 runs of minimum-difficulty headers must not prevent a + // proof from reaching a later decisive header. The Bridge consumes the + // full header chain, so the maintainer must do the same. After 144 DIFF1 + // headers, the previous-epoch difficulty 16 header binds the requested + // difficulty and brings the observed total above 6*16=96. + "decisive header follows long minimum difficulty run": { + transactionConfirmations: 145, + currentEpochDifficulty: diff(32), + previousEpochDifficulty: diff(16), + headerDifficultyAt: func(h uint) *big.Int { + if h < proofStart+144 { + return diff(1) + } + return diff(16) + }, + headersFrom: proofStart, + headersTo: proofStart + 144, + + expectedIsProofWithinRelayRange: true, + expectedAccumulatedConfirmations: 145, + expectedRequiredConfirmations: 145, + }, // The chain tip is reached before enough difficulty is accumulated. // The reported requirement is one header more than currently exists, // so the caller waits for more confirmations. @@ -191,13 +236,21 @@ func TestGetProofInfo(t *testing.T) { localChain := newLocalChain() btcChain := newLocalBitcoinChain() - if err := populateBlockHeaders( - btcChain, - test.headersFrom, - test.headersTo, - test.headerDifficultyAt, - ); err != nil { - t.Fatal(err) + if test.headerAt != nil { + for h := test.headersFrom; h <= test.headersTo; h++ { + if err := btcChain.addBlockHeader(h, test.headerAt(h)); err != nil { + t.Fatal(err) + } + } + } else { + if err := populateBlockHeaders( + btcChain, + test.headersFrom, + test.headersTo, + test.headerDifficultyAt, + ); err != nil { + t.Fatal(err) + } } btcChain.addTransactionConfirmations( transactionHash, @@ -221,7 +274,18 @@ func TestGetProofInfo(t *testing.T) { localChain, localChain, ) - if err != nil { + if test.expectedError != "" { + if err == nil { + t.Fatalf("expected error containing [%v]", test.expectedError) + } + if !strings.Contains(err.Error(), test.expectedError) { + t.Fatalf( + "unexpected error\nexpected to contain: [%v]\nactual: [%v]", + test.expectedError, + err, + ) + } + } else if err != nil { t.Fatal(err) }