search

iqlusioninc / liquidity-staking-module

Other
87 stars 29 forks source link

ICA liquid stake Delegation is not accounted for validator TotalLiquidShares #118

Closed xlab closed 1 year ago

xlab commented 1 year ago

Hello! Finally had time to dig this issue I've got from this ibc-go v7 test:

FAIL TestKeeperTestSuite/TestOnRecvPacket/interchain_account_successfully_executes_stakingtypes.MsgDelegate_and_stakingtypes.MsgUndelegate_sequentially

            Error:          Received unexpected error:
                            validator liquid shares underflow

Turns out in this piece of code https://github.com/iqlusioninc/liquidity-staking-module/blob/1e00e3c6750819f8be5cc01067583d34d5a0c7f4/x/staking/keeper/msg_server.go#L239-L245 the validator is passed as a copy, so while its .TotalLiquidShares value gets updated and set in the keeper, the subsequent call to .Keeper.Delegate() with a verbatim copy of non-updated validator structure overrides that.

Basically any test where ICA delegates and then tries to undelegate will trigger an underflow, because TotalLiquidShares wasn't updated.

I've made necessary changes to pass validator by reference instead, and also added a test TestICADelegateUndelegate that provides the necessary coverage.

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