From 4aaf122719ef121cff7274f1478f41413a0dfbc3 Mon Sep 17 00:00:00 2001 From: libotony Date: Tue, 19 Nov 2024 21:30:18 +0800 Subject: [PATCH] return error in newSequence (#885) * return error in newSequence * revert type change of sequence * adjust sequence to 63bit * fix test --------- Co-authored-by: otherview --- api/events/types.go | 13 +++---- api/events/types_test.go | 73 +++++++++++++++++++++------------------- logdb/logdb.go | 51 +++++++++++++++++++++------- logdb/sequence.go | 26 ++++++++------ logdb/sequence_test.go | 28 ++++++--------- 5 files changed, 111 insertions(+), 80 deletions(-) diff --git a/api/events/types.go b/api/events/types.go index 2a107ed54..6cecfe9a8 100644 --- a/api/events/types.go +++ b/api/events/types.go @@ -174,17 +174,18 @@ func ConvertRange(chain *chain.Chain, r *Range) (*logdb.Range, error) { }, nil } - // Units are blocks, locked a max of 28bits in the logdb + // Units are block numbers - numbers will have a max ceiling at chain head block number + headNum := block.Number(chain.HeadID()) from := uint32(r.From) to := uint32(r.To) - // Ensure the values are capped at the 28-bit maximum - if uint32(r.From) > logdb.BlockNumMask { - from = logdb.BlockNumMask + if from > headNum { + from = headNum } - if uint32(r.To) > logdb.BlockNumMask { - to = logdb.BlockNumMask + if to > headNum { + to = headNum } + return &logdb.Range{ From: from, To: to, diff --git a/api/events/types_test.go b/api/events/types_test.go index 75eafe3a7..e223bb158 100644 --- a/api/events/types_test.go +++ b/api/events/types_test.go @@ -11,17 +11,17 @@ import ( "github.com/ethereum/go-ethereum/common/hexutil" "github.com/stretchr/testify/assert" - "github.com/vechain/thor/v2/chain" + "github.com/stretchr/testify/require" "github.com/vechain/thor/v2/genesis" "github.com/vechain/thor/v2/logdb" - "github.com/vechain/thor/v2/muxdb" - "github.com/vechain/thor/v2/state" + "github.com/vechain/thor/v2/test/testchain" "github.com/vechain/thor/v2/thor" + "github.com/vechain/thor/v2/tx" ) func TestEventsTypes(t *testing.T) { c := initChain(t) - for name, tt := range map[string]func(*testing.T, *chain.Chain){ + for name, tt := range map[string]func(*testing.T, *testchain.Chain){ "testConvertRangeWithBlockRangeType": testConvertRangeWithBlockRangeType, "testConvertRangeWithTimeRangeTypeLessThenGenesis": testConvertRangeWithTimeRangeTypeLessThenGenesis, "testConvertRangeWithTimeRangeType": testConvertRangeWithTimeRangeType, @@ -33,21 +33,38 @@ func TestEventsTypes(t *testing.T) { } } -func testConvertRangeWithBlockRangeType(t *testing.T, chain *chain.Chain) { +func testConvertRangeWithBlockRangeType(t *testing.T, chain *testchain.Chain) { rng := &Range{ Unit: BlockRangeType, From: 1, To: 2, } - convertedRng, err := ConvertRange(chain, rng) + convertedRng, err := ConvertRange(chain.Repo().NewBestChain(), rng) assert.NoError(t, err) assert.Equal(t, uint32(rng.From), convertedRng.From) assert.Equal(t, uint32(rng.To), convertedRng.To) + + // ensure wild block numbers have a max ceiling of chain.head + rng = &Range{ + Unit: BlockRangeType, + From: 100, + To: 200, + } + + convertedRng, err = ConvertRange(chain.Repo().NewBestChain(), rng) + require.NoError(t, err) + + bestBlock, err := chain.BestBlock() + require.NoError(t, err) + + assert.NoError(t, err) + assert.Equal(t, bestBlock.Header().Number(), convertedRng.From) + assert.Equal(t, bestBlock.Header().Number(), convertedRng.To) } -func testConvertRangeWithTimeRangeTypeLessThenGenesis(t *testing.T, chain *chain.Chain) { +func testConvertRangeWithTimeRangeTypeLessThenGenesis(t *testing.T, chain *testchain.Chain) { rng := &Range{ Unit: TimeRangeType, From: 1, @@ -58,17 +75,15 @@ func testConvertRangeWithTimeRangeTypeLessThenGenesis(t *testing.T, chain *chain To: math.MaxUint32, } - convRng, err := ConvertRange(chain, rng) + convRng, err := ConvertRange(chain.Repo().NewBestChain(), rng) assert.NoError(t, err) assert.Equal(t, expectedEmptyRange, convRng) } -func testConvertRangeWithTimeRangeType(t *testing.T, chain *chain.Chain) { - genesis, err := chain.GetBlockHeader(0) - if err != nil { - t.Fatal(err) - } +func testConvertRangeWithTimeRangeType(t *testing.T, chain *testchain.Chain) { + genesis := chain.GenesisBlock().Header() + rng := &Range{ Unit: TimeRangeType, From: 1, @@ -79,17 +94,15 @@ func testConvertRangeWithTimeRangeType(t *testing.T, chain *chain.Chain) { To: 0, } - convRng, err := ConvertRange(chain, rng) + convRng, err := ConvertRange(chain.Repo().NewBestChain(), rng) assert.NoError(t, err) assert.Equal(t, expectedZeroRange, convRng) } -func testConvertRangeWithFromGreaterThanGenesis(t *testing.T, chain *chain.Chain) { - genesis, err := chain.GetBlockHeader(0) - if err != nil { - t.Fatal(err) - } +func testConvertRangeWithFromGreaterThanGenesis(t *testing.T, chain *testchain.Chain) { + genesis := chain.GenesisBlock().Header() + rng := &Range{ Unit: TimeRangeType, From: genesis.Timestamp() + 1_000, @@ -100,29 +113,21 @@ func testConvertRangeWithFromGreaterThanGenesis(t *testing.T, chain *chain.Chain To: math.MaxUint32, } - convRng, err := ConvertRange(chain, rng) + convRng, err := ConvertRange(chain.Repo().NewBestChain(), rng) assert.NoError(t, err) assert.Equal(t, expectedEmptyRange, convRng) } // Init functions -func initChain(t *testing.T) *chain.Chain { - muxDb := muxdb.NewMem() - stater := state.NewStater(muxDb) - gene := genesis.NewDevnet() - - b, _, _, err := gene.Build(stater) - if err != nil { - t.Fatal(err) - } +func initChain(t *testing.T) *testchain.Chain { + thorChain, err := testchain.NewIntegrationTestChain() + require.NoError(t, err) - repo, err := chain.NewRepository(muxDb, b) - if err != nil { - t.Fatal(err) - } + require.NoError(t, thorChain.MintBlock(genesis.DevAccounts()[0], []*tx.Transaction{}...)) + require.NoError(t, thorChain.MintBlock(genesis.DevAccounts()[0], []*tx.Transaction{}...)) - return repo.NewBestChain() + return thorChain } func TestConvertEvent(t *testing.T) { diff --git a/logdb/logdb.go b/logdb/logdb.go index 610a2cac1..af043e9d2 100644 --- a/logdb/logdb.go +++ b/logdb/logdb.go @@ -117,13 +117,18 @@ FROM (%v) e if filter.Range != nil { subQuery += " AND seq >= ?" - if filter.Range.To > BlockNumMask { - return nil, fmt.Errorf("invalid block number range") + from, err := newSequence(filter.Range.From, 0, 0) + if err != nil { + return nil, err } - args = append(args, newSequence(filter.Range.From, 0, 0)) + args = append(args, from) if filter.Range.To >= filter.Range.From { subQuery += " AND seq <= ?" - args = append(args, newSequence(filter.Range.To, txIndexMask, logIndexMask)) + to, err := newSequence(filter.Range.To, txIndexMask, logIndexMask) + if err != nil { + return nil, err + } + args = append(args, to) } } @@ -186,13 +191,18 @@ FROM (%v) t if filter.Range != nil { subQuery += " AND seq >= ?" - if filter.Range.To > BlockNumMask { - return nil, fmt.Errorf("invalid block number range") + from, err := newSequence(filter.Range.From, 0, 0) + if err != nil { + return nil, err } - args = append(args, newSequence(filter.Range.From, 0, 0)) + args = append(args, from) if filter.Range.To >= filter.Range.From { subQuery += " AND seq <= ?" - args = append(args, newSequence(filter.Range.To, txIndexMask, logIndexMask)) + to, err := newSequence(filter.Range.To, txIndexMask, logIndexMask) + if err != nil { + return nil, err + } + args = append(args, to) } } @@ -383,7 +393,10 @@ func (db *LogDB) HasBlockID(id thor.Bytes32) (bool, error) { UNION SELECT * FROM (SELECT seq FROM event WHERE seq=? AND blockID=` + refIDQuery + ` LIMIT 1))` - seq := newSequence(block.Number(id), 0, 0) + seq, err := newSequence(block.Number(id), 0, 0) + if err != nil { + return false, err + } row := db.stmtCache.MustPrepare(query).QueryRow(seq, id[:], seq, id[:]) var count int if err := row.Scan(&count); err != nil { @@ -433,7 +446,11 @@ type Writer struct { // Truncate truncates the database by deleting logs after blockNum (included). func (w *Writer) Truncate(blockNum uint32) error { - seq := newSequence(blockNum, 0, 0) + seq, err := newSequence(blockNum, 0, 0) + if err != nil { + return err + } + if err := w.exec("DELETE FROM event WHERE seq >= ?", seq); err != nil { return err } @@ -526,9 +543,14 @@ func (w *Writer) Write(b *block.Block, receipts tx.Receipts) error { eventData = ev.Data } + seq, err := newSequence(blockNum, uint32(txIndex), eventCount) + if err != nil { + return err + } + if err := w.exec( query, - newSequence(blockNum, uint32(txIndex), eventCount), + seq, blockTimestamp, clauseIndex, eventData, @@ -561,9 +583,14 @@ func (w *Writer) Write(b *block.Block, receipts tx.Receipts) error { refIDQuery + "," + refIDQuery + ")" + seq, err := newSequence(blockNum, uint32(txIndex), transferCount) + if err != nil { + return err + } + if err := w.exec( query, - newSequence(blockNum, uint32(txIndex), transferCount), + seq, blockTimestamp, clauseIndex, tr.Amount.Bytes(), diff --git a/logdb/sequence.go b/logdb/sequence.go index efe41e55d..1e98458b7 100644 --- a/logdb/sequence.go +++ b/logdb/sequence.go @@ -5,38 +5,42 @@ package logdb +import "errors" + type sequence int64 // Adjust these constants based on your bit allocation requirements +// 64th bit is the sign bit so we have 63 bits to use const ( blockNumBits = 28 txIndexBits = 15 - logIndexBits = 21 - // BlockNumMask Max = 2^28 - 1 = 268,435,455 (unsigned int 28) - BlockNumMask = (1 << blockNumBits) - 1 + logIndexBits = 20 + // Max = 2^28 - 1 = 268,435,455 (unsigned int 28) + blockNumMask = (1 << blockNumBits) - 1 // Max = 2^15 - 1 = 32,767 txIndexMask = (1 << txIndexBits) - 1 - // Max = 2^21 - 1 = 2,097,151 + // Max = 2^20 - 1 = 1,048,575 logIndexMask = (1 << logIndexBits) - 1 ) -func newSequence(blockNum uint32, txIndex uint32, logIndex uint32) sequence { - if blockNum > BlockNumMask { - panic("block number too large") +func newSequence(blockNum uint32, txIndex uint32, logIndex uint32) (sequence, error) { + if blockNum > blockNumMask { + return 0, errors.New("block number out of range: uint28") } if txIndex > txIndexMask { - panic("transaction index too large") + return 0, errors.New("tx index out of range: uint15") } if logIndex > logIndexMask { - panic("log index too large") + return 0, errors.New("log index out of range: uint21") } + return (sequence(blockNum) << (txIndexBits + logIndexBits)) | (sequence(txIndex) << logIndexBits) | - sequence(logIndex) + sequence(logIndex), nil } func (s sequence) BlockNumber() uint32 { - return uint32(s>>(txIndexBits+logIndexBits)) & BlockNumMask + return uint32(s>>(txIndexBits+logIndexBits)) & blockNumMask } func (s sequence) TxIndex() uint32 { diff --git a/logdb/sequence_test.go b/logdb/sequence_test.go index 6442ad689..2855447f9 100644 --- a/logdb/sequence_test.go +++ b/logdb/sequence_test.go @@ -7,6 +7,8 @@ package logdb import ( "testing" + + "github.com/stretchr/testify/assert" ) func TestSequence(t *testing.T) { @@ -20,14 +22,20 @@ func TestSequence(t *testing.T) { args args }{ {"regular", args{1, 2, 3}}, - {"max bn", args{BlockNumMask, 1, 2}}, + {"max bn", args{blockNumMask, 1, 2}}, {"max tx index", args{5, txIndexMask, 4}}, {"max log index", args{5, 4, logIndexMask}}, - {"both max", args{BlockNumMask, txIndexMask, logIndexMask}}, + {"close to max", args{blockNumMask - 5, txIndexMask - 5, logIndexMask - 5}}, + {"both max", args{blockNumMask, txIndexMask, logIndexMask}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := newSequence(tt.args.blockNum, tt.args.txIndex, tt.args.logIndex) + got, err := newSequence(tt.args.blockNum, tt.args.txIndex, tt.args.logIndex) + if err != nil { + t.Error(err) + } + + assert.True(t, got > 0, "sequence should be positive") if bn := got.BlockNumber(); bn != tt.args.blockNum { t.Errorf("seq.blockNum() = %v, want %v", bn, tt.args.blockNum) } @@ -39,18 +47,4 @@ func TestSequence(t *testing.T) { } }) } - - defer func() { - if e := recover(); e == nil { - t.Errorf("newSequence should panic on 2nd arg > math.MaxInt32") - } - }() - newSequence(1, txIndexMask+1, 5) - - defer func() { - if e := recover(); e == nil { - t.Errorf("newSequence should panic on 3rd arg > math.MaxInt32") - } - }() - newSequence(1, 5, logIndexMask+1) }