-
Notifications
You must be signed in to change notification settings - Fork 209
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
chore(state-transition): cleaned up state in state-transition UTs #2518
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,23 +58,25 @@ type ( | |
TestStateProcessorT = core.StateProcessor | ||
) | ||
|
||
type testKVStoreService struct { | ||
ctx sdk.Context | ||
} | ||
type testKVStoreService struct{} | ||
|
||
func (kvs *testKVStoreService) OpenKVStore(context.Context) corestore.KVStore { | ||
//nolint:contextcheck // fine with tests | ||
store := sdk.UnwrapSDKContext(kvs.ctx).KVStore(testStoreKey) | ||
func (kvs *testKVStoreService) OpenKVStore(ctx context.Context) corestore.KVStore { | ||
store := sdk.UnwrapSDKContext(ctx).KVStore(testStoreKey) | ||
return storage.NewKVStore(store) | ||
} | ||
|
||
//nolint:gochecknoglobals // unexported and use only in tests | ||
//nolint:gochecknoglobals // unexported and used only in tests | ||
var testStoreKey = storetypes.NewKVStoreKey("state-transition-tests") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the purpose of this special key? iirc the key by default in prod is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rezbera I could have reused the prod one, but when I first built these tests I wanted to make sure I fully controlled them and I could use a different one. |
||
|
||
func initTestStores() (*beacondb.KVStore, *depositstore.KVStore, error) { | ||
func BuildTestStores() ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pre-emptively exporting |
||
storetypes.CommitMultiStore, | ||
*beacondb.KVStore, | ||
*depositstore.KVStore, | ||
error, | ||
) { | ||
db, err := db.OpenDB("", dbm.MemDBBackend) | ||
if err != nil { | ||
return nil, nil, fmt.Errorf("failed opening mem db: %w", err) | ||
return nil, nil, nil, fmt.Errorf("failed opening mem db: %w", err) | ||
} | ||
var ( | ||
nopLog = log.NewNopLogger() | ||
|
@@ -90,12 +92,12 @@ func initTestStores() (*beacondb.KVStore, *depositstore.KVStore, error) { | |
|
||
cms.MountStoreWithDB(testStoreKey, storetypes.StoreTypeIAVL, nil) | ||
if err = cms.LoadLatestVersion(); err != nil { | ||
return nil, nil, fmt.Errorf("failed to load latest version: %w", err) | ||
return nil, nil, nil, fmt.Errorf("failed to load latest version: %w", err) | ||
} | ||
|
||
ctx := sdk.NewContext(cms, true, nopLog) | ||
testStoreService := &testKVStoreService{ctx: ctx} | ||
return beacondb.New(testStoreService), | ||
testStoreService := &testKVStoreService{} | ||
return cms, | ||
beacondb.New(testStoreService), | ||
depositstore.NewStore(testStoreService, noopCloseFunc, nopLog), | ||
nil | ||
} | ||
|
@@ -118,9 +120,11 @@ func SetupTestState(t *testing.T, cs chain.Spec) ( | |
|
||
dummyProposerAddr := []byte{0xff} | ||
|
||
kvStore, depositStore, err := initTestStores() | ||
cms, kvStore, depositStore, err := BuildTestStores() | ||
require.NoError(t, err) | ||
beaconState := statedb.NewBeaconStateFromDB(kvStore, cs) | ||
|
||
sdkCtx := sdk.NewContext(cms.CacheMultiStore(), true, log.NewNopLogger()) | ||
beaconState := statedb.NewBeaconStateFromDB(kvStore.WithContext(sdkCtx), cs) | ||
|
||
sp := core.NewStateProcessor( | ||
noop.NewLogger[any](), | ||
|
@@ -135,7 +139,7 @@ func SetupTestState(t *testing.T, cs chain.Spec) ( | |
) | ||
|
||
ctx := transition.NewTransitionCtx( | ||
context.Background(), | ||
sdkCtx, | ||
0, // time | ||
dummyProposerAddr, | ||
). | ||
|
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.
this was a first attempt at creating state but it is wrong.
We should use the context provided by the transactions and extract the state frorm there, not use and reuse a single context.
It worked so far because we never handled rejected blocks or multiple blocks for the same height, which we need in upcoming tests.