Skip to content
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

Upstream changes from iqlusioninc:v0.45.16-ics-lsm #6

Merged
merged 5 commits into from
Jul 21, 2023

Conversation

xlab
Copy link

@xlab xlab commented Jul 18, 2023

Intro

As LSM prepares to be merged into Cosmos SDK v45 (ICS) branch, the last two weeks were heavy on some last-moment changes and improvements on Cosmos SDK side. I am glad that this implementation gets many new eyes from SDK team and most intricate issues are being tackled in the upstream.

This PR brings most recent and important changes from the upstream branch iqlusioninc:v0.45.16-ics-lsm, as well as some pending PRs from iqlusioninc repo. Obviously all changes that I took there are being manually adapted for 0.47, tests fixed and lots of issues that I spotted along the way were reported to the upstream. At this moment, I suppose we have 1:1 compatibility with store protos, internal logic, APIs and definitions of the LSM version that goes into SDK.

Since I got all tests working, it seems to be the time we could cut off a release for our Persistence Testnet v8. Once this PR is merged, we can bump all dependent versions.

Note: This PR better be reviewed commit-by-commit!

Testing

Unit tests

Testing all individual modules with unit tests:

cd x/
go test ./...

Integration tests

cd tests/
make test-integration

Runs longer, expected results

ok  	github.com/cosmos/cosmos-sdk/tests/integration/bank/keeper	67.340s
ok  	github.com/cosmos/cosmos-sdk/tests/integration/distribution	0.094s
ok  	github.com/cosmos/cosmos-sdk/tests/integration/distribution/keeper	0.678s
ok  	github.com/cosmos/cosmos-sdk/tests/integration/evidence/keeper	0.094s
ok  	github.com/cosmos/cosmos-sdk/tests/integration/genutil	0.241s
ok  	github.com/cosmos/cosmos-sdk/tests/integration/gov	0.224s
ok  	github.com/cosmos/cosmos-sdk/tests/integration/gov/keeper	0.643s
ok  	github.com/cosmos/cosmos-sdk/tests/integration/runtime	0.124s
ok  	github.com/cosmos/cosmos-sdk/tests/integration/slashing/keeper	0.193s
ok  	github.com/cosmos/cosmos-sdk/tests/integration/staking/keeper	122.156s
ok  	github.com/cosmos/cosmos-sdk/tests/integration/store/rootmulti	0.119s

E2E built-in tests

cd tests/
make test-e2e

Runs some time, expected results

ok  	github.com/cosmos/cosmos-sdk/tests/e2e/auth	109.854s
ok  	github.com/cosmos/cosmos-sdk/tests/e2e/auth/vesting	49.149s
ok  	github.com/cosmos/cosmos-sdk/tests/e2e/authz	216.283s
ok  	github.com/cosmos/cosmos-sdk/tests/e2e/bank	44.782s
ok  	github.com/cosmos/cosmos-sdk/tests/e2e/client/grpc/tmservice	20.544s
ok  	github.com/cosmos/cosmos-sdk/tests/e2e/crisis	24.790s
ok  	github.com/cosmos/cosmos-sdk/tests/e2e/distribution	110.377s
ok  	github.com/cosmos/cosmos-sdk/tests/e2e/evidence	20.511s
ok  	github.com/cosmos/cosmos-sdk/tests/e2e/feegrant	96.403s
ok  	github.com/cosmos/cosmos-sdk/tests/e2e/genutil	20.926s
ok  	github.com/cosmos/cosmos-sdk/tests/e2e/gov	143.318s
ok  	github.com/cosmos/cosmos-sdk/tests/e2e/group	636.578s
ok  	github.com/cosmos/cosmos-sdk/tests/e2e/mint	20.025s
ok  	github.com/cosmos/cosmos-sdk/tests/e2e/nft	31.890s
ok  	github.com/cosmos/cosmos-sdk/tests/e2e/params	21.164s
ok  	github.com/cosmos/cosmos-sdk/tests/e2e/server	1.831s
ok  	github.com/cosmos/cosmos-sdk/tests/e2e/slashing	24.183s
ok  	github.com/cosmos/cosmos-sdk/tests/e2e/staking	332.557s
ok  	github.com/cosmos/cosmos-sdk/tests/e2e/tx	63.113s
ok  	github.com/cosmos/cosmos-sdk/tests/e2e/upgrade	17.282s

Simulation

Runs from the project root dir:

make test-sim-nondeterminism

Runs some time, expected results

--- PASS: TestAppStateDeterminism (63.65s)
PASS
ok  	cosmossdk.io/simapp	63.702s

Optionally, to achieve the fullest testing possible:

make test-sim-import-export

External Tests

There is a branch in persistenceCore persistenceOne/persistenceCore#220 that implements some E2E LSM tests that interact with x/gov proposal while tokenizing. Make sure to clone the repo at the PR's branch and run the testing:

make local-image
make ictest-lsm

If everything works fine, both tests will finish in parallel in about 1m30s:

--- PASS: TestTokenizeSendVote (89.31s)
--- PASS: TestMultiTokenizeVote (93.91s)
PASS
ok  	github.com/persistenceOne/persistenceCore/v8/interchaintest	94.339s

List of changes

I understand that it's hard to comprehend changes by glancing at the wall of text in the diff, and it's even harder to map the changes in my single commit there to wall of commits (103 commits!!! 😓) in the upstream branch. Therefore I put a list of changes that we apply in this PR. It is just a memory dump.

  • fixed precision issues by representing LSM tokens 1:1 to delegation shares (see PR)
  • fixed a bug with tokenize/redeem using ICA (validator struct was passed as copy)
  • changed slashed token calculation
  • liquid shares underflow checks
  • added CancelTokenizeShareLockExpiration to remove the lock from the queue
  • AccountIsLiquidStakingProvider renamed to DelegatorIsLiquidStaker with more documentation
  • agreed on common names for Ledger's Amino encoding (x/distribution, x/staking)
  • agreed on common error codes for x/distribution and x/staking
  • agreed on store keys and prefixes for x/staking
  • TokenizeShareRecordReward grpc query fixed bug
  • rewrote some of the proto comments
  • TokenizeShareLockStatus definition moved to staking proto file
  • renamed total_validator_bond_shares into validator_bond_shares everywhere
  • renamed total_liquid_shares into liquid_shares everywhere
  • deprecate min_self_delegation field in proto
  • removed min_self_delegation from most interfaces and tests
  • removed related tests like TestRedelegateSelfDelegation
  • change some staking query namings in proto
  • total number of liquid staked tokens in genesis state
  • tokenize share locks in genesis state
  • LastTokenizeShareRecordId in genesis state
  • genesis import/export updates
  • re-gen and format proto
  • update ADR-61 doc text
  • x/staking store migration with defaults for LSM
  • adapt and fix integration tests
  • adapt and fix e2e tests
  • added stakingtypes.ModuleName into simapp config
  • new e2e/distribution test for NewWithdrawAllTokenizeShareRecordRewardCmd
  • new e2e test helpers
  • new staking/genesis integration tests
  • simulation tests fixes and nondeterminism issues resolve
  • added additional operations into simulation test
  • randomly generate validatorBondFactor, globalLiquidStakingCap, validatorLiquidStakingCap during sims

@xlab xlab requested a review from ajeet97 July 18, 2023 03:17
@xlab xlab force-pushed the max/v47-lsm-upstream-changes branch from c642c50 to 3d77d2b Compare July 19, 2023 20:51
@xlab xlab force-pushed the max/v47-lsm-upstream-changes branch from 3d77d2b to e57af28 Compare July 19, 2023 20:57
@xlab xlab marked this pull request as ready for review July 20, 2023 01:20
@xlab xlab requested review from puneet2019 and hieuvubk July 20, 2023 01:20
puneet2019
puneet2019 previously approved these changes Jul 20, 2023
ajeet97
ajeet97 previously approved these changes Jul 20, 2023
hieuvubk
hieuvubk previously approved these changes Jul 21, 2023
@xlab xlab dismissed stale reviews from hieuvubk, ajeet97, and puneet2019 via 091d502 July 21, 2023 14:34
@xlab xlab requested review from ajeet97 and puneet2019 July 21, 2023 14:37
@xlab xlab merged commit 27b53e5 into v47-lsm Jul 21, 2023
@xlab xlab deleted the max/v47-lsm-upstream-changes branch July 21, 2023 17:51
ajeet97 pushed a commit that referenced this pull request Mar 20, 2024
Upstream changes from iqlusioninc:v0.45.16-ics-lsm
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants