feat: add LSM to the SDK's v0.45.16-ics-lsm branch

[riley-stride]

Description

Add Iqlusion's LSM to the SDK in v0.45.16-ics-lsm for use in the upcoming Cosmoshub v11 release.

Please see this Iqlusion forum post for a full description of the LSM deployment plan on the Cosmoshub.

TLDR; On this branch:

• ICS changes are managed by the ICS/Hub team
• LSM changes are managed by Iqlusion team
• SDK changes (bugfixes expected only) are managed by the SDK team

Credit to @sampocs for a lot of hard work combining ICS + LSM 🙌

---

Disclaimers for reviewers:

1. There's one last stubborn failing unit test: TestGRPCServer_Reflection (LOC). The unit test complains that the TokenizeShareRecord struct isn't accessible, but it's defined here.

Error log

--- FAIL: TestIntegrationTestSuite (105.88s)
    server_test.go:45: setting up integration test suite
    network.go:172: acquiring test network lock
    network.go:177: created temporary directory: /var/folders/m5/12pnd05500n56z31y9rtmmsr0000gn/T/TestIntegrationTestSuite2846596830/001/chain-D7Uv8k3239444353
    network.go:186: preparing test network...
    network.go:379: starting test network...
    network.go:384: started test network
    --- FAIL: TestIntegrationTestSuite/TestGRPCServer_Reflection (89.70s)
        server_test.go:126: 
            	Error Trace:	/Users/rileyedmunds/iqlusion/cosmos-sdk/server/grpc/server_test.go:126
            	Error:      	Received unexpected error:
            	            	file "cosmos/staking/v1beta1/query.proto" included an unresolvable reference to ".cosmos.staking.v1beta1.TokenizeShareRecord"
            	Test:       	TestIntegrationTestSuite/TestGRPCServer_Reflection

2. Linting fails on this branch. However, linting also fails on the v0.45.16-ics branch. We believe we've resolved all lint issues related to the diffs that are added in this pull request.

3. MsgCancelUnbondingDelegation was part of the original LSM design since LSM was originally written on SDK46. We ported it over here as a safety feature, but are open to removing it if the SDK team prefers keeping 45 fully feature-frozen.

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...

• [ ] included the correct type prefix in the PR title
• [ ] added ! to the type prefix if API or client breaking change
• [ ] targeted the correct branch (see PR Targeting)
• [ ] provided a link to the relevant issue or specification
• [ ] followed the guidelines for building modules
• [ ] included the necessary unit and integration tests
• [ ] added a changelog entry to CHANGELOG.md
• [ ] included comments for documenting Go code
• [ ] updated the relevant documentation or specification
• [ ] reviewed "Files changed" and left comments if necessary
• [ ] confirmed all CI checks have passed

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...

• [ ] confirmed the correct type prefix in the PR title
• [ ] confirmed ! in the type prefix if API or client breaking change
• [ ] confirmed all author checklist items have been addressed
• [ ] reviewed state machine logic
• [ ] reviewed API design and naming
• [ ] reviewed documentation is accurate
• [ ] reviewed tests and test coverage
• [ ] manually tested (if applicable)

[zmanian]

Reviewed. Looks good.

Some observations

1. In the iqlusion implementation, we removed min-self-delegation completely from the code. This code simply hardcodes the value to zero in the interface. I think that's okay. Just wanted to note it.

2. This code doesn't appear to contain any migration code from the original x/staking implantation unless I missed it.

Otherwise, looks good.

[zmanian]

A fix for this bug needs to be included here

https://github.com/iqlusioninc/liquidity-staking-module/pull/117

[sampocs]

@zmanian

This code doesn't appear to contain any migration code from the original x/staking implantation unless I missed it.

Should the migration code live here or in gaia?

[riley-stride]

Added three LSM ADRs. ADR001 contains a link to a full spec. Credit to @sampocs for the ADRs and spec 🙌

[mpoke]

@zmanian

This code doesn't appear to contain any migration code from the original x/staking implantation unless I missed it.

Should the migration code live here or in gaia?

I think here. The migration is usually part of the module.

[sampocs]

@mpoke Couple questions on the migration:

1. Please double check me on this, but since the protos only have fields added, I don't think we have to increment the module's consensus version, "register" the migration, or store down and deserialize using the old types (i.e. we can just use the keepers from the latest binary to both deserialize and reserialize)
2. Should the params be set to the default "disabled" value here and then overridden with the gaia values in gaia's upgrade handler?
3. Where should the migration functions live? I see the legacy directory but looks like it wasn't touched in 2 years. Was there no migration necessary for ICS?

[mpoke]

@sampocs

Was there no migration necessary for ICS?

No as ICS didn't affect any of the existing state. ICS affects only new unbonding operations.

Where should the migration functions live? I see the legacy directory but looks like it wasn't touched in 2 years.

I don't think the staking module was changed in a while. @alexanderbez could you please confirm?

Should the params be set to the default "disabled" value here and then overridden with the gaia values in gaia's upgrade handler?

The new params require migration even if they are set to the default value. Again, @alexanderbez could you please confirm?

Please double check me on this, but since the protos only have fields added, I don't think we have to increment the module's consensus version, "register" the migration, or store down and deserialize using the old types (i.e. we can just use the keepers from the latest binary to both deserialize and reserialize)

If the params require migration, this means we need to increment the module's consensus version. Regarding the changes in the protos, I'm not sure. @MSalopek what do you think?

[sampocs]

@mpoke

The new params require migration even if they are set to the default value

Sorry, my phrasing was a bit confusing. I didn't mean to imply that they would not be migrated. By default values, I meant the "disabled" values (i.e. -1 for bond factor and 1.0 for global/LS cap). Not to be confused with the zero values. For gaia, these should be set to 250 and 0.25. My question was whether we should set them to 250/0.25 in this module's migration code, or whether we should set them to -1/1.0 in this migration code and then override it in the gaia upgrade handler.

re: consensus version, now that I think about it, I think we'll have to increment the version in order to force the migration code (I was conflating this with chain upgrades where you have a bit more freedom when it comes to how to upgrade code is executed)

[xlab]

@sampocs

re: consensus version, now that I think about it, I think we'll have to increment the version in order to force the migration code

Yes, but if the version is bumped to v3 or something, it will collide with already existing upgrade path v3 -> v4 https://github.com/cosmos/cosmos-sdk/blob/f3e4697195ff2c62a8843460f99cae19d3adab6f/x/staking/module.go#L159

So far I don't understand how to make a consensus version specifically for LSM on v45, but without affecting upgrade paths for v46 onwards.

[sampocs]

@xlab Hmm, very good point. One option is to just register it as 25 (i.e. 2.5)

It also feels a bit weird to register this migration with the module considering this is really just a branch of the SDK meant for gaia - rather than a new version that will be used by other chains. I'm wondering if it makes more sense to just write the migration functions, but not register them, and then call those from the gaia upgrade handler

[sampocs]

@reviewers - the distribution changes were accidentally merged into the wrong base branch. pulling those in now

[mpoke]

nit: could somebody run make format?

[mpoke]

Also, are we sure we cannot get more tests passing? @alexanderbez Are any of the CI checks failing a problem?

[zmanian]

LFG

[julienrbrt]

I haven't reviewed this, but is it expected that those sims are failing:

• https://github.com/cosmos/cosmos-sdk/actions/runs/5856588630/job/15877096737?pr=16747
• https://github.com/cosmos/cosmos-sdk/actions/runs/5856588630/job/15877096454?pr=16747
• https://github.com/cosmos/cosmos-sdk/actions/runs/5856588630/job/15877095843?pr=16747

See snippet

$ go test ./simapp -run TestFullAppSimulation -Enabled=true -NumBlocks=50 -Genesis= -Verbose=true -Commit=true -Seed=32 -Period=10 -v -timeout 24h
....
Starting the simulation from time Mon Aug 22 13:36:50 UTC 8839 (unixtime 216784906610)
Simulating... block 20/50, operation 250/259. simulation halted due to panic on block 20
Logs to writing to /home/julien/.simapp/simulations/2023-08-15_18:44:55.log
--- FAIL: TestFullAppSimulation (6.19s)
panic: negative coin amount [recovered]
        panic: negative coin amount [recovered]
        panic: negative coin amount

goroutine 51 [running]:
testing.tRunner.func1.2({0x139b820, 0x1da88e0})
        /usr/local/go/src/testing/testing.go:1545 +0x238
testing.tRunner.func1()
        /usr/local/go/src/testing/testing.go:1548 +0x397
panic({0x139b820?, 0x1da88e0?})
        /usr/local/go/src/runtime/panic.go:914 +0x21f
github.com/cosmos/cosmos-sdk/x/simulation.SimulateFromSeed.func2()
        /home/julien/projects/cosmos/cosmos-sdk/x/simulation/simulate.go:146 +0xd3
panic({0x139b820?, 0x1da88e0?})
        /usr/local/go/src/runtime/panic.go:920 +0x270
github.com/cosmos/cosmos-sdk/types.DecCoins.Sub(...)
        /home/julien/projects/cosmos/cosmos-sdk/types/dec_coin.go:306
github.com/cosmos/cosmos-sdk/x/distribution/keeper.Keeper.WithdrawValidatorCommission({{0x1dafc00, 0xc00011f1b0}, {0x7fd474320f58, 0xc00018f6d0}, {{0x7fd474320f58, 0xc00018f6d0}, 0xc00011aa18, {0x1dafc00, 0xc00011f1e0}, {0x1dafc50, ...}, ...}, ...}, ...)
        /home/julien/projects/cosmos/cosmos-sdk/x/distribution/keeper/keeper.go:119 +0x890
github.com/cosmos/cosmos-sdk/x/distribution/keeper.RegisterInvariants.CanWithdrawInvariant.func2.1(0x15c4600?, {0x1dd04f8, 0xc001915440})
        /home/julien/projects/cosmos/cosmos-sdk/x/distribution/keeper/invariants.go:80 +0xd8
github.com/cosmos/cosmos-sdk/x/staking/keeper.Keeper.IterateValidators({{0x1dafc00, 0xc00011f190}, {0x7fd474320f58, 0xc00018f6d0}, {0x1dc0458, 0xc000304ea0}, {0x7fd474320fc8, 0xc000217340}, {0x1dc9770, 0xc000156a80}, ...}, ...)
        /home/julien/projects/cosmos/cosmos-sdk/x/staking/keeper/alias_functions.go:23 +0x1ec
github.com/cosmos/cosmos-sdk/x/distribution/keeper.RegisterInvariants.CanWithdrawInvariant.func2({{0x1dbf620, 0x28ebfe0}, {0x1dc8c10, 0xc00357db00}, {{0x0, 0x0}, {0x15e51cb, 0xe}, 0x14, {0x0, ...}, ...}, ...})
        /home/julien/projects/cosmos/cosmos-sdk/x/distribution/keeper/invariants.go:79 +0x51c
github.com/cosmos/cosmos-sdk/x/crisis/keeper.Keeper.AssertInvariants({{0xc0002a0c80, 0xb, 0x10}, {{0x7fd474320f58, 0xc00018f6d0}, 0xc00011aa18, {0x1dafc00, 0xc00011f1e0}, {0x1dafc50, 0xc00011f250}, ...}, ...}, ...)
        /home/julien/projects/cosmos/cosmos-sdk/x/crisis/keeper/keeper.go:79 +0x41d
github.com/cosmos/cosmos-sdk/x/crisis.EndBlocker({{0x1dbf620, 0x28ebfe0}, {0x1dc8c10, 0xc00357db00}, {{0x0, 0x0}, {0x15e51cb, 0xe}, 0x14, {0x0, ...}, ...}, ...}, ...)
        /home/julien/projects/cosmos/cosmos-sdk/x/crisis/abci.go:20 +0x218
github.com/cosmos/cosmos-sdk/x/crisis.AppModule.EndBlock(...)
        /home/julien/projects/cosmos/cosmos-sdk/x/crisis/module.go:165
github.com/cosmos/cosmos-sdk/types/module.(*Manager).EndBlock(_, {{0x1dbf620, 0x28ebfe0}, {0x1dc8c10, 0xc00357db00}, {{0x0, 0x0}, {0x15e51cb, 0xe}, 0x14, ...}, ...}, ...)
        /home/julien/projects/cosmos/cosmos-sdk/types/module/module.go:512 +0x1eb
github.com/cosmos/cosmos-sdk/simapp.(*SimApp).EndBlocker(...)
        /home/julien/projects/cosmos/cosmos-sdk/simapp/app.go:436
github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).EndBlock(0xc000148700, {0x28ebfe0?})
        /home/julien/projects/cosmos/cosmos-sdk/baseapp/abci.go:203 +0x1fd
github.com/cosmos/cosmos-sdk/x/simulation.SimulateFromSeed({0x1dce3f0?, 0xc0005829c0?}, {0x1dab540?, 0xc00008e030}, 0x0?, _, _, {0xc0002a6900, 0x1f, 0x30}, ...)
        /home/julien/projects/cosmos/cosmos-sdk/x/simulation/simulate.go:185 +0x17d8
github.com/cosmos/cosmos-sdk/simapp.TestFullAppSimulation(0xc0005829c0)
        /home/julien/projects/cosmos/cosmos-sdk/simapp/sim_test.go:74 +0x5d4
testing.tRunner(0xc0005829c0, 0x1c27e98)
        /usr/local/go/src/testing/testing.go:1595 +0xff
created by testing.(*T).Run in goroutine 1
        /usr/local/go/src/testing/testing.go:1648 +0x3ad
FAIL    github.com/cosmos/cosmos-sdk/simapp     6.216s
FAIL