-
Notifications
You must be signed in to change notification settings - Fork 117
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
perf(state-transition): reduce amount of withdrawals processed #2110
Open
abi87
wants to merge
28
commits into
main
Choose a base branch
from
fix-maximum-number-withdrawals
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+686
−57
Open
Changes from all commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
b4a1077
skip zero amount withdrawals to reduce number of withdrawals processed
abi87 f4a7d79
wip: adding UTs to state transition package
abi87 0f46172
wip: completed simple UT for state transition package
abi87 22c7717
wip: minimal execution engine stub
abi87 05cba80
extended asserts
abi87 443ac1b
added test case
abi87 10efd5e
nits
abi87 099716d
tests for helpers in state transition using mock
nidhi-singh02 151a533
Revert "tests for helpers in state transition using mock"
nidhi-singh02 9818c7e
tests with only mock for execution engine
nidhi-singh02 160cc88
removed test for VerifyAndNotifyNewPayload
nidhi-singh02 4a9fe1c
nit
abi87 e048be4
improved unit tests asserts
abi87 8bf34db
appease linter
abi87 d90a95a
fix(state-transition): fix deposit index upon genesis processing (#2116)
abi87 e17d29c
fixed bad merge
abi87 8dfe59d
Merge branch 'main' into fix-maximum-number-withdrawals
abi87 daa9262
Merge branch 'state-transition-add-UTs' into fix-maximum-number-withd…
abi87 92d494f
added UTs to show no withdrawals are made when unnecessary
abi87 6286b20
Merge branch 'main' into state-transition-add-UTs
abi87 a1a3def
Merge branch 'state-transition-add-UTs' into fix-maximum-number-withd…
abi87 97fba5c
fix withdrawal index update
abi87 af8c5e0
fix(build): erigon repo
gummybera 023ebfd
fix(build): bump erigon to recent version
gummybera d66b298
nits from code review
abi87 420f621
Merge branch 'state-transition-add-UTs' into fix-maximum-number-withd…
abi87 09eee8e
Merge branch 'fix-erigon' into fix-maximum-number-withdrawals
abi87 d8a267e
hacked fix for withdrawals SSZ serialization
abi87 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,138 @@ | ||
// SPDX-License-Identifier: BUSL-1.1 | ||
// | ||
// Copyright (C) 2024, Berachain Foundation. All rights reserved. | ||
// Use of this software is governed by the Business Source License included | ||
// in the LICENSE file of this repository and at www.mariadb.com/bsl11. | ||
// | ||
// ANY USE OF THE LICENSED WORK IN VIOLATION OF THIS LICENSE WILL AUTOMATICALLY | ||
// TERMINATE YOUR RIGHTS UNDER THIS LICENSE FOR THE CURRENT AND ALL OTHER | ||
// VERSIONS OF THE LICENSED WORK. | ||
// | ||
// THIS LICENSE DOES NOT GRANT YOU ANY RIGHT IN ANY TRADEMARK OR LOGO OF | ||
// LICENSOR OR ITS AFFILIATES (PROVIDED THAT YOU MAY USE A TRADEMARK OR LOGO OF | ||
// LICENSOR AS EXPRESSLY REQUIRED BY THIS LICENSE). | ||
// | ||
// TO THE EXTENT PERMITTED BY APPLICABLE LAW, THE LICENSED WORK IS PROVIDED ON | ||
// AN “AS IS” BASIS. LICENSOR HEREBY DISCLAIMS ALL WARRANTIES AND CONDITIONS, | ||
// EXPRESS OR IMPLIED, INCLUDING (WITHOUT LIMITATION) WARRANTIES OF | ||
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE, NON-INFRINGEMENT, AND | ||
// TITLE. | ||
|
||
package core_test | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
|
||
corestore "cosmossdk.io/core/store" | ||
"cosmossdk.io/log" | ||
"cosmossdk.io/store" | ||
"cosmossdk.io/store/metrics" | ||
storetypes "cosmossdk.io/store/types" | ||
"github.com/berachain/beacon-kit/mod/consensus-types/pkg/types" | ||
engineprimitives "github.com/berachain/beacon-kit/mod/engine-primitives/pkg/engine-primitives" | ||
"github.com/berachain/beacon-kit/mod/node-core/pkg/components" | ||
statedb "github.com/berachain/beacon-kit/mod/state-transition/pkg/core/state" | ||
"github.com/berachain/beacon-kit/mod/storage/pkg/beacondb" | ||
"github.com/berachain/beacon-kit/mod/storage/pkg/db" | ||
"github.com/berachain/beacon-kit/mod/storage/pkg/encoding" | ||
dbm "github.com/cosmos/cosmos-db" | ||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
) | ||
|
||
type ( | ||
TestBeaconStateMarshallableT = types.BeaconState[ | ||
*types.BeaconBlockHeader, | ||
*types.Eth1Data, | ||
*types.ExecutionPayloadHeader, | ||
*types.Fork, | ||
*types.Validator, | ||
types.BeaconBlockHeader, | ||
types.Eth1Data, | ||
types.ExecutionPayloadHeader, | ||
types.Fork, | ||
types.Validator, | ||
] | ||
|
||
TestKVStoreT = beacondb.KVStore[ | ||
*types.BeaconBlockHeader, | ||
*types.Eth1Data, | ||
*types.ExecutionPayloadHeader, | ||
*types.Fork, | ||
*types.Validator, | ||
types.Validators, | ||
] | ||
|
||
TestBeaconStateT = statedb.StateDB[ | ||
*types.BeaconBlockHeader, | ||
*TestBeaconStateMarshallableT, | ||
*types.Eth1Data, | ||
*types.ExecutionPayloadHeader, | ||
*types.Fork, | ||
*TestKVStoreT, | ||
*types.Validator, | ||
types.Validators, | ||
*engineprimitives.Withdrawal, | ||
types.WithdrawalCredentials, | ||
] | ||
) | ||
|
||
type testKVStoreService struct { | ||
ctx sdk.Context | ||
} | ||
|
||
func (kvs *testKVStoreService) OpenKVStore(context.Context) corestore.KVStore { | ||
//nolint:contextcheck // fine with tests | ||
return components.NewKVStore( | ||
sdk.UnwrapSDKContext(kvs.ctx).KVStore(testStoreKey), | ||
) | ||
} | ||
|
||
var ( | ||
testStoreKey = storetypes.NewKVStoreKey("state-transition-tests") | ||
testCodec = &encoding.SSZInterfaceCodec[*types.ExecutionPayloadHeader]{} | ||
) | ||
|
||
func initTestStore() ( | ||
*beacondb.KVStore[ | ||
*types.BeaconBlockHeader, | ||
*types.Eth1Data, | ||
*types.ExecutionPayloadHeader, | ||
*types.Fork, | ||
*types.Validator, | ||
types.Validators, | ||
], error) { | ||
db, err := db.OpenDB("", dbm.MemDBBackend) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed opening mem db: %w", err) | ||
} | ||
var ( | ||
nopLog = log.NewNopLogger() | ||
nopMetrics = metrics.NewNoOpMetrics() | ||
) | ||
|
||
cms := store.NewCommitMultiStore( | ||
db, | ||
nopLog, | ||
nopMetrics, | ||
) | ||
|
||
ctx := sdk.NewContext(cms, true, nopLog) | ||
cms.MountStoreWithDB(testStoreKey, storetypes.StoreTypeIAVL, nil) | ||
if err = cms.LoadLatestVersion(); err != nil { | ||
return nil, fmt.Errorf("failed to load latest version: %w", err) | ||
} | ||
testStoreService := &testKVStoreService{ctx: ctx} | ||
|
||
return beacondb.New[ | ||
*types.BeaconBlockHeader, | ||
*types.Eth1Data, | ||
*types.ExecutionPayloadHeader, | ||
*types.Fork, | ||
*types.Validator, | ||
types.Validators, | ||
]( | ||
testStoreService, | ||
testCodec, | ||
), nil | ||
} |
86 changes: 86 additions & 0 deletions
86
mod/state-transition/pkg/core/mocks/execution_engine.mock.go
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
post Shangai an EL explicitly check that Withdrawals are not nil (instead empty slices are fine).
Currently BeaconKit duly builds a block with Withdrawals set to empty slice (if not withdrawals are available) but as soon as the block is returned by CometBFT for verification, the SSZ decoding sets the empty slice to nil.
This code change solves the issue.