Skip to content

Commit

Permalink
fix: fix index out of range undeterministic error in rpc test (#3718)
Browse files Browse the repository at this point in the history
  • Loading branch information
P1sar committed Jan 22, 2024
1 parent 51d57ae commit d099384
Show file tree
Hide file tree
Showing 5 changed files with 734 additions and 26 deletions.
38 changes: 17 additions & 21 deletions dot/state/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@ var (
// BlockState contains the historical block data of the blockchain, including block headers and bodies.
// It wraps the blocktree (which contains unfinalised blocks) and the database (which contains finalised blocks).
type BlockState struct {
bt *blocktree.BlockTree
baseState *BaseState
dbPath string
db BlockStateDatabase
sync.RWMutex
bt *blocktree.BlockTree
baseState *BaseState
dbPath string
db BlockStateDatabase
lock sync.RWMutex
genesisHash common.Hash
lastFinalised common.Hash
unfinalisedBlocks *hashToBlockMap
Expand Down Expand Up @@ -334,7 +334,7 @@ func (bs *BlockState) GetBlockHashesBySlot(slotNum uint64) ([]common.Hash, error
return nil, fmt.Errorf("failed to get descendants: %w", err)
}

blocksWithGivenSlot := []common.Hash{}
var blocksWithGivenSlot []common.Hash

for _, desc := range descendants {
descSlot, err := bs.GetSlotForBlock(desc)
Expand Down Expand Up @@ -380,8 +380,8 @@ func (bs *BlockState) GetBlockByNumber(num uint) (*types.Block, error) {

// GetBlockByHash returns a block for a given hash
func (bs *BlockState) GetBlockByHash(hash common.Hash) (*types.Block, error) {
bs.RLock()
defer bs.RUnlock()
bs.lock.RLock()
defer bs.lock.RUnlock()

block := bs.unfinalisedBlocks.getBlock(hash)
if block != nil {
Expand Down Expand Up @@ -413,8 +413,8 @@ func (bs *BlockState) SetHeader(header *types.Header) error {

// HasBlockBody returns true if the db contains the block body
func (bs *BlockState) HasBlockBody(hash common.Hash) (bool, error) {
bs.RLock()
defer bs.RUnlock()
bs.lock.RLock()
defer bs.lock.RUnlock()

if bs.unfinalisedBlocks.getBlock(hash) != nil {
return true, nil
Expand Down Expand Up @@ -471,8 +471,8 @@ func (bs *BlockState) CompareAndSetBlockData(bd *types.BlockData) error {

// AddBlock adds a block to the blocktree and the DB with arrival time as current unix time
func (bs *BlockState) AddBlock(block *types.Block) error {
bs.Lock()
defer bs.Unlock()
bs.lock.Lock()
defer bs.lock.Unlock()
return bs.AddBlockWithArrivalTime(block, time.Now())
}

Expand Down Expand Up @@ -678,8 +678,7 @@ func (bs *BlockState) retrieveRange(startHash, endHash common.Hash) (hashes []co
return nil, fmt.Errorf("retrieving range from database: %w", err)
}

hashes = append(inDatabaseHashes, inMemoryHashes...)
return hashes, nil
return append(inDatabaseHashes, inMemoryHashes...), nil
}

var ErrStartHashMismatch = errors.New("start hash mismatch")
Expand All @@ -706,11 +705,8 @@ func (bs *BlockState) retrieveRangeFromDatabase(startHash common.Hash,

lastPosition := blocksInRange - 1

hashes[0] = startHash
hashes[lastPosition] = endHeader.Hash()

inLoopHash := endHeader.ParentHash
for currentPosition := lastPosition - 1; currentPosition > 0; currentPosition-- {
inLoopHash := endHeader.Hash()
for currentPosition := int(lastPosition); currentPosition >= 0; currentPosition-- {
hashes[currentPosition] = inLoopHash

inLoopHeader, err := bs.loadHeaderFromDatabase(inLoopHash)
Expand All @@ -721,9 +717,9 @@ func (bs *BlockState) retrieveRangeFromDatabase(startHash common.Hash,
inLoopHash = inLoopHeader.ParentHash
}

// here we ensure that we finished up the loop
// here we ensure that we finished up the loop with the hash we used as start
// with the same hash as the startHash
if inLoopHash != startHash {
if hashes[0] != startHash {
return nil, fmt.Errorf("%w: expecting %s, found: %s", ErrStartHashMismatch, startHash.Short(), inLoopHash.Short())
}

Expand Down
8 changes: 4 additions & 4 deletions dot/state/block_finalisation.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ func (bs *BlockState) NumberIsFinalised(num uint) (bool, error) {

// GetFinalisedHeader returns the finalised block header by round and setID
func (bs *BlockState) GetFinalisedHeader(round, setID uint64) (*types.Header, error) {
bs.Lock()
defer bs.Unlock()
bs.lock.Lock()
defer bs.lock.Unlock()

h, err := bs.GetFinalisedHash(round, setID)
if err != nil {
Expand Down Expand Up @@ -116,8 +116,8 @@ func (bs *BlockState) GetHighestFinalisedHeader() (*types.Header, error) {

// SetFinalisedHash sets the latest finalised block hash
func (bs *BlockState) SetFinalisedHash(hash common.Hash, round, setID uint64) error {
bs.Lock()
defer bs.Unlock()
bs.lock.Lock()
defer bs.lock.Unlock()

has, err := bs.HasHeader(hash)
if err != nil {
Expand Down
32 changes: 32 additions & 0 deletions dot/state/block_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1099,3 +1099,35 @@ func Test_GetRuntime_StoreRuntime(t *testing.T) {
require.NoError(t, err)
require.Equal(t, runtimeInstance, sameRuntimeOnDiffHash)
}

const headerHex = "0x276bfa91f70859348285599321ea96afd3ae681f0be47d36196bac8075ea32e804496b5fbd26d7014ba2ef84b588859428e334549d374726263ea894691ff78a0d34aea7c0b7cdadce2f44389a28d4200e522cec26d4eae828183ce40bd57ebeee0c0642414245b50103000000002792f1100000000068ab0948ad2f3194f94c4a584bb8dc118d354316c10a51e04ce4ca7aed4a971c046b5dc4704d9fa43daad797291efa00a1070970f979d4593888dbcb7a628b08667ca7a2850eed67f908d9fce9ec45e6e688341d054d3f074ec102ae3844ca0f044241424529010104d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d01000000000000000000000000000000000000000000000000000000000000000000000000000000054241424501016425bfe39b70f23a0de6b4e8159cf513c25cb7e18f52c6ba889f426211bb787dbe0f0a237cc93052f9fa8a68386722299b6c8ba0d8060ad6a1cbdb9688e9a982" //nolint:lll
//nolint:lll
// This decodes to next Header -> ParentHash=0x276bfa91f70859348285599321ea96afd3ae681f0be47d36196bac8075ea32e8 Number=1 StateRoot=0x496b5fbd26d7014ba2ef84b588859428e334549d374726263ea894691ff78a0d ExtrinsicsRoot=0x34aea7c0b7cdadce2f44389a28d4200e522cec26d4eae828183ce40bd57ebeee Digest=[PreRuntimeDigest ConsensusEngineID=BABE Data=0x03000000002792f1100000000068ab0948ad2f3194f94c4a584bb8dc118d354316c10a51e04ce4ca7aed4a971c046b5dc4704d9fa43daad797291efa00a1070970f979d4593888dbcb7a628b08667ca7a2850eed67f908d9fce9ec45e6e688341d054d3f074ec102ae3844ca0f, ConsensusDigest ConsensusEngineID=BABE Data=0x0104d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d01000000000000000000000000000000000000000000000000000000000000000000000000000000, SealDigest ConsensusEngineID=BABE Data=0x6425bfe39b70f23a0de6b4e8159cf513c25cb7e18f52c6ba889f426211bb787dbe0f0a237cc93052f9fa8a68386722299b6c8ba0d8060ad6a1cbdb9688e9a982] Hash=0x885d464b8c8753f88973cfa457d385c8fe9c9d90a6a2f62e013cf81de3666812 //nolint:lll

func Test_retrieveRangeFromDatabaseWithOneBlock(t *testing.T) {
ctrl := gomock.NewController(t)

telemetryMock := NewMockTelemetry(ctrl)
telemetryMock.EXPECT().SendMessage(gomock.Any()).AnyTimes()
db := NewInMemoryDB(t)
genesisHeader := &types.Header{
Number: 0,
StateRoot: trie.EmptyHash,
Digest: types.NewDigest(),
}
bs, err := NewBlockStateFromGenesis(db, newTriesEmpty(), genesisHeader, telemetryMock)
require.NoError(t, err)

headerBytes, err := common.HexToBytes(headerHex)
require.NoError(t, err)

headerType := types.NewEmptyHeader()
err = scale.Unmarshal(headerBytes, headerType)
require.NoError(t, err)
err = bs.SetHeader(headerType)
require.NoError(t, err)

hashes, err := bs.retrieveRangeFromDatabase(headerType.Hash(), headerType)
require.NoError(t, err)
require.Equal(t, len(hashes), 1)
}

0 comments on commit d099384

Please sign in to comment.