Skip to content
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

fix: fix index out of range undeterministic error in rpc test #3718

Merged
merged 6 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
}
Loading
Loading