-
Notifications
You must be signed in to change notification settings - Fork 226
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
Use cosmos-sdk DefaultBaseappOptions
instead of replicating
#9425
Comments
We definitely need some testing to avoid further regressions here. @gibson042 in #9426 (review)_:
|
closes: #9424 ## Description Plumb the iavl options into the cosmos baseapp. Copied by comparing to `DefaultBaseappOptions` in cosmos-sdk `server/util.go`, which we should likely use directly instead (see #9425), but this is a minimal change we can easily adopt now. ### Security Considerations None ### Scaling Considerations None ### Documentation Considerations None. Change should explicitly be part of the release notes ### Testing Considerations Unsure about how to test this ### Upgrade Considerations Chain software, not consensus affecting.
My understanding is that this is only needed for operation that write to the DB, but I may be mistaken |
closes: #9100 closes: #9425 ## Description Until we have better layering of the snapshots extensions in cosmos-sdk, this replaces the implementation of the `snapshots export` command to initiate the snapshot creation through our front-running mechanism used for state-sync. Drive-by cleanup of the root command as well to address #9425 ### Security Considerations None, ### Scaling Considerations None ### Documentation Considerations Communicate to validators that we fixed the command and that it's usable as expected with any cosmos chain. ### Testing Considerations Added an integration test since this only modifies the command line handling Manually tested with the following ``` cd packages/cosmic-swingset make scenario2-setup make scenario2-run-chain # wait until there are blocks, then kill agd --home t1/n0 snapshots export rm -rf t1/n0/data/application.db t1/n0/data/agoric # the above removes app state without removing cometbft state as that cannot # be restored easily in a single node chain agd --home t1/n0 snapshot restore $snapshot_height 2 make scenario2-run-chain # verify blocks are being produced ``` ### Upgrade Considerations Since we replace the upstream sdk command handling, any future changes upstream would have to be reflected in the override.
closes: #9424 ## Description Plumb the iavl options into the cosmos baseapp. Copied by comparing to `DefaultBaseappOptions` in cosmos-sdk `server/util.go`, which we should likely use directly instead (see #9425), but this is a minimal change we can easily adopt now. ### Security Considerations None ### Scaling Considerations None ### Documentation Considerations None. Change should explicitly be part of the release notes ### Testing Considerations Unsure about how to test this ### Upgrade Considerations Chain software, not consensus affecting.
closes: #9100 closes: #9425 Until we have better layering of the snapshots extensions in cosmos-sdk, this replaces the implementation of the `snapshots export` command to initiate the snapshot creation through our front-running mechanism used for state-sync. Drive-by cleanup of the root command as well to address #9425 None, None Communicate to validators that we fixed the command and that it's usable as expected with any cosmos chain. Added an integration test since this only modifies the command line handling Manually tested with the following ``` cd packages/cosmic-swingset make scenario2-setup make scenario2-run-chain agd --home t1/n0 snapshots export rm -rf t1/n0/data/application.db t1/n0/data/agoric agd --home t1/n0 snapshot restore $snapshot_height 2 make scenario2-run-chain ``` Since we replace the upstream sdk command handling, any future changes upstream would have to be reflected in the override.
What is the Problem Being Solved?
We recently missed some iavl options that were introduced by an cosmos sdk upgrade because our
appCreator
didn't add support for these (#9424). Instead of manually adding support for these, we should leverage the cosmos-sdkDefaultBaseappOptions
helper inserver/util.go
.Description of the Design
Remove custom logic in
appCreator.newApp
, and adoptDefaultBaseappOptions
. Consider adopting inappCreator.appExport
as well.Security Considerations
None
Scaling Considerations
Less manual tracking of upstream changes
Test Plan
TBD
Upgrade Considerations
None
The text was updated successfully, but these errors were encountered: