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.

Sign up for GitHub

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 VM.GetBlockIDAtHeight #595

Merged
merged 3 commits into from
Jul 8, 2024
Merged

Fix VM.GetBlockIDAtHeight #595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c30bf89
Fix and optimize GetBlockIDAtHeight
StephenButtolph Jul 7, 2024
ae92c8f
Add test
StephenButtolph Jul 7, 2024
7512324
Merge branch 'master' into fix-get-block-id-at-height
darioush Jul 8, 2024
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
23 changes: 12 additions & 11 deletions plugin/evm/vm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1397,20 +1397,21 @@ func (vm *VM) VerifyHeightIndex(context.Context) error {
return nil
}

// GetBlockAtHeight returns the canonical block at [blkHeight].
// If [blkHeight] is less than the height of the last accepted block, this will return
// the block accepted at that height. Otherwise, it may return a blkID that has not yet
// been accepted.
// Note: the engine assumes that if a block is not found at [blkHeight], then
// [database.ErrNotFound] will be returned. This indicates that the VM has state synced
// and does not have all historical blocks available.
func (vm *VM) GetBlockIDAtHeight(_ context.Context, blkHeight uint64) (ids.ID, error) {
ethBlock := vm.blockChain.GetBlockByNumber(blkHeight)
if ethBlock == nil {
// GetBlockAtHeight returns the canonical block at [height].
// Note: the engine assumes that if a block is not found at [height], then
// [database.ErrNotFound] will be returned. This indicates that the VM has state
// synced and does not have all historical blocks available.
func (vm *VM) GetBlockIDAtHeight(_ context.Context, height uint64) (ids.ID, error) {
lastAcceptedBlock := vm.LastAcceptedBlock()
if lastAcceptedBlock.Height() < height {
return ids.ID{}, database.ErrNotFound
}

return ids.ID(ethBlock.Hash()), nil
hash := vm.blockChain.GetCanonicalHash(height)
if hash == (common.Hash{}) {
return ids.ID{}, database.ErrNotFound
}
return ids.ID(hash), nil
}

func (vm *VM) Version(context.Context) (string, error) {
Expand Down
31 changes: 31 additions & 0 deletions plugin/evm/vm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2021,6 +2021,10 @@ func TestNonCanonicalAccept(t *testing.T) {
t.Fatalf("Expected status of built block to be %s, but found %s", choices.Processing, status)
}

if _, err := vm1.GetBlockIDAtHeight(context.Background(), vm1BlkA.Height()); err != database.ErrNotFound {
t.Fatalf("Expected unaccepted block not to be indexed by height, but found %s", err)
}

if err := vm1.SetPreference(context.Background(), vm1BlkA.ID()); err != nil {
t.Fatal(err)
}
Expand All @@ -2035,16 +2039,29 @@ func TestNonCanonicalAccept(t *testing.T) {
if status := vm2BlkA.Status(); status != choices.Processing {
t.Fatalf("Expected status of block on VM2 to be %s, but found %s", choices.Processing, status)
}
if _, err := vm2.GetBlockIDAtHeight(context.Background(), vm2BlkA.Height()); err != database.ErrNotFound {
t.Fatalf("Expected unaccepted block not to be indexed by height, but found %s", err)
}
if err := vm2.SetPreference(context.Background(), vm2BlkA.ID()); err != nil {
t.Fatal(err)
}

if err := vm1BlkA.Accept(context.Background()); err != nil {
t.Fatalf("VM1 failed to accept block: %s", err)
}
if blkID, err := vm1.GetBlockIDAtHeight(context.Background(), vm1BlkA.Height()); err != nil {
t.Fatalf("Height lookuped failed on accepted block: %s", err)
} else if blkID != vm1BlkA.ID() {
t.Fatalf("Expected accepted block to be indexed by height, but found %s", blkID)
}
if err := vm2BlkA.Accept(context.Background()); err != nil {
t.Fatalf("VM2 failed to accept block: %s", err)
}
if blkID, err := vm2.GetBlockIDAtHeight(context.Background(), vm2BlkA.Height()); err != nil {
t.Fatalf("Height lookuped failed on accepted block: %s", err)
} else if blkID != vm2BlkA.ID() {
t.Fatalf("Expected accepted block to be indexed by height, but found %s", blkID)
}

newHead := <-newTxPoolHeadChan1
if newHead.Head.Hash() != common.Hash(vm1BlkA.ID()) {
Expand Down Expand Up @@ -2092,6 +2109,10 @@ func TestNonCanonicalAccept(t *testing.T) {
t.Fatalf("Expected status of built block to be %s, but found %s", choices.Processing, status)
}

if _, err := vm1.GetBlockIDAtHeight(context.Background(), vm1BlkB.Height()); err != database.ErrNotFound {
t.Fatalf("Expected unaccepted block not to be indexed by height, but found %s", err)
}

if err := vm1.SetPreference(context.Background(), vm1BlkB.ID()); err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -2126,10 +2147,20 @@ func TestNonCanonicalAccept(t *testing.T) {
t.Fatalf("Block failed verification on VM1: %s", err)
}

if _, err := vm1.GetBlockIDAtHeight(context.Background(), vm1BlkC.Height()); err != database.ErrNotFound {
t.Fatalf("Expected unaccepted block not to be indexed by height, but found %s", err)
}

if err := vm1BlkC.Accept(context.Background()); err != nil {
t.Fatalf("VM1 failed to accept block: %s", err)
}

if blkID, err := vm1.GetBlockIDAtHeight(context.Background(), vm1BlkC.Height()); err != nil {
t.Fatalf("Height lookuped failed on accepted block: %s", err)
} else if blkID != vm1BlkC.ID() {
t.Fatalf("Expected accepted block to be indexed by height, but found %s", blkID)
}

blkCHash := vm1BlkC.(*chain.BlockWrapper).Block.(*Block).ethBlock.Hash()
if b := vm1.blockChain.GetBlockByNumber(blkBHeight); b.Hash() != blkCHash {
t.Fatalf("expected block at %d to have hash %s but got %s", blkBHeight, blkCHash.Hex(), b.Hash().Hex())
Expand Down
Loading