return error in newSequence (#885)

* return error in newSequence

* revert type change of sequence

* adjust sequence to 63bit

* fix test

---------

Co-authored-by: otherview <[email protected]>

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,

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) {

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(),

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 {

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)
 }