-
Notifications
You must be signed in to change notification settings - Fork 48
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
build(deps): bump cosmos-sdk, cometbft cometbft-db #1124
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1124 +/- ##
=======================================
Coverage 80.92% 80.93%
=======================================
Files 194 198 +4
Lines 16901 16966 +65
=======================================
+ Hits 13677 13731 +54
- Misses 2629 2638 +9
- Partials 595 597 +2
☔ View full report in Codecov by Sentry. |
// Create export folder | ||
if config.ExportStatePath != "" { | ||
err = os.MkdirAll(path.Dir(config.ExportStatePath), 0755) | ||
require.NoError(t, err) | ||
} | ||
if config.ExportParamsPath != "" { | ||
println(path.Dir(config.ExportParamsPath)) | ||
err = os.MkdirAll(path.Dir(config.ExportParamsPath), 0755) | ||
require.NoError(t, err) | ||
} |
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.
Since the CheckExportSimulation
assumes the folders exist, we should make sure that they are present before being used. This should solve any possible problem about folders not existing
@@ -248,8 +258,6 @@ func TestAppImportExport(t *testing.T) { | |||
{app.keys[poststypes.StoreKey], newApp.keys[poststypes.StoreKey], [][]byte{}}, | |||
{app.keys[reportstypes.StoreKey], newApp.keys[reportstypes.StoreKey], [][]byte{}}, | |||
{app.keys[reactionstypes.StoreKey], newApp.keys[reactionstypes.StoreKey], [][]byte{}}, | |||
|
|||
{app.keys[wasm.StoreKey], newApp.keys[wasm.StoreKey], [][]byte{wasmtypes.TXCounterPrefix}}, |
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.
If we don't remove this, some tests might fail with the following error:
compared 2 different key/value pairs between KVStoreKey{0xc001159ba0, wasm} and KVStoreKey{0xc00327f700, wasm}
sim_test.go:275:
Error Trace: /home/riccardo/Coding/Desmos/Desmos/app/sim_test.go:275
Error: Not equal:
expected: 2
actual : 0
Test: TestAppImportExport
Messages: store A =>
store B lastContractId =>
store A =>
store B =>
This is the case of a test that can be reproduced by running
cd app && go test . -run TestAppImportExport -Enabled=true -NumBlocks=50 -Genesis= -Verbose=true -Commit=true -Seed=981928 -Period=5 -ExportParamsPath /tmp/sim-logs-2838039267/sim_params-981928.json -ExportStatePath /tmp/sim-logs-2838039267/sim_state-981928.json -v -timeout 24h
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.
It is because the x/wasm
simulation does not generate the proper LastInstanceID
for genesis state. I created an issue on wasmd repo. I think we can add it back after the issue is fixed.
CosmWasm/wasmd#1372
@@ -144,8 +143,6 @@ func (suite *TestSuite) SetupTest() { | |||
suite.ctx = sdk.NewContext(ms, tmproto.Header{ChainID: "test-chain"}, false, log.NewNopLogger()) | |||
suite.cdc, suite.legacyAminoCdc = app.MakeCodecs() | |||
|
|||
paramsKeeper := paramskeeper.NewKeeper(suite.cdc, suite.legacyAminoCdc, keys[paramstypes.StoreKey], tKeys[paramstypes.TStoreKey]) |
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 and all other paramsKeeper
are no longer necessary, so we should remove them to make sure the linting passes
test-race: ARGS=-race -tags='cgo ledger test_ledger_mock' | ||
test-unit: test_tags += cgo ledger test_ledger_mock norace | ||
test-unit-amino: test_tags += ledger test_ledger_mock test_amino norace | ||
test-ledger: test_tags += cgo ledger norace | ||
test-ledger-mock: test_tags += ledger test_ledger_mock norace | ||
test-race: test_tags += cgo ledger test_ledger_mock | ||
test-race: ARGS=-race |
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.
These changes are to make sure we use the same scheme as the Cosmos SDK so it's going to be easier for us to check any differences in the future
concurrency: | ||
group: ci-${{ github.ref }}-sims | ||
cancel-in-progress: true |
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.
Taken from the Cosmos SDK repo, it aviods any concurrency between simulation tests. It should make sure we don't have multiple simulation tests running for the same PR/branch/commit at the same time
github.com/cosmos/cosmos-proto v1.0.0-beta.2 | ||
github.com/cosmos/cosmos-sdk v0.47.1 | ||
github.com/cosmos/cosmos-sdk v0.47.2 |
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.
It is would be better to upgrade desmos-cosmos-sdk to 0.47.2 as well.
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.
Thanks, updated it!
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.
LGTM
Description
This PR
github.com/cosmos/cosmos-sdk
tov0.47.2
github.com/cometbft/cometbft
tov0.37.1
github.com/cometbft/cometbft-db
tov0.8.0
Closes #1122
Closes #1123
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking change