chore: Cosmos SDK 0.44.* series upgrade for Osmosis #538
Conversation
…osis into faddat/sdk-0441
faddat
changed the title
…osis into faddat/sdk-0441
|
I'm now keeping two cosmos-sdk branches fresh in osmosis-labs/cosmos-sdk:
For testing purposes, I am also tagging and releasing them so that we can build osmosis against it. This opens up many possibilities. I figure that the best possible time of transition would be:
|
|
Thanks so much for doing this! The merge conflicts here were a ton of work! Going to start reviewing this tomorrow. Quick notes, I think we should keep Can you provide some context for the decisions with |
|
Sure! So basically I figured that the airdrop command was now a part of history. By traveling back throught the repository someone could always verify the airdrop, and that is what is important about it. I also think that I needed to import some.... v036 somesuch thing-- and that had been dropped from v0.44.2 so I also dropped the airdrop command and figured folks could grab it if they wanted it. also-- do we want authz? also, also: if we were to upgrade, would we export state and begin anew as osmosis-2 (my preference because we would gain many performance improvements) or do you reckon we should do another in-place upgrade? |
|
@faddat what kind of performance upgrades do we get from an export state upgrade? |
|
@sunnya97 the performance improvements tend to be quite dramatic. That said, we surely cannot lose the existing IBC channels, so I will look into other ways to roll. I'm also going to look into weather it is realistic to include this in v5.x |
|
I think its realistic to include this in v5.x! We made a branch for the changes by cherry-picking every commit manually instead of the rebase hell. (Spent awhile trying to figure out how to double check everything in a rebase and its super hard) I think we can use If your fine with it, can I just push to this branch directly? I think it should take some minor changes before this is merge ready! |
| AppConstructor: NewAppConstructor(encCfg), | ||
| GenesisState: ModuleBasics.DefaultGenesis(encCfg.Marshaler), | ||
| TimeoutCommit: 2 * time.Second, | ||
| ChainID: "osmosis-code-test", |
There was a problem hiding this comment.
TODO: Double check this still gets set correctly in client.toml after osmosisd init
There was a problem hiding this comment.
chain id is set under the logic:
"chainID, _ := cmd.Flags().GetString(flags.FlagChainID)
if chainID == "" {
chainID = fmt.Sprintf("test-chain-%v", tmrand.Str(6))
}"
as far as I can tell.
There was a problem hiding this comment.
Ah no, client.toml happens entirely separately. I'll file an issue for updating that.
Codecov Report
@@ Coverage Diff @@
## main #538 +/- ##
==========================================
- Coverage 20.23% 18.43% -1.81%
==========================================
Files 172 172
Lines 24172 24203 +31
==========================================
- Hits 4892 4462 -430
- Misses 18486 18977 +491
+ Partials 794 764 -30
Continue to review full report at Codecov.
|
|
All tests now working. Now need to fix simulation, and the IBC upgrade guide to this version |
| // This is a hardcoded path in the events to get the lockID | ||
| // this is incredibly brittle... | ||
| // fmt.Println(txResp.Logs[0]) | ||
| lockID := txResp.Logs[0].Events[2].Attributes[0].Value |
There was a problem hiding this comment.
Need to open an issue for this
| suite.Assert().EqualValues(122316, int(avgGas), "average gas / lock") | ||
| suite.Assert().EqualValues(242903, int(maxGas), "max gas / lock") |
There was a problem hiding this comment.
These constants are suspected to have gone up due to FundAccount instead of SetBalances. This is actually noise we need to remove from the Lockup Gas tests
|
Done, opening follow-up issues now! |
Closes: #419
Description
This is ready for review. One thing that'll need review especially is the decimal math stuff in the faddat/basecoin cosmos sdk. I am working to get that up to 0.44.2 and when that's done, the sdk stuff will need review also.
Thanks!
Tracking checklist:
Cherry pick(merge?) individual commits on Osmo-v2-upgrade into 0.44.2 branch <- maybe-complete.NB: This has also added & enabled authz and feegrant. It should also ease some of @sunnya97's work on ethereum address formats.
Big big Goal
Osmo is IBC yo.
oh, and I hate, hate hate Marshaler.
For contributor use:
docs/) or specification (x/<module>/spec/)Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorer