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

Inflation Fix #951

Merged
merged 12 commits into from
Oct 14, 2023
Merged

Inflation Fix #951

merged 12 commits into from
Oct 14, 2023

Conversation

shellvish
Copy link
Contributor

@shellvish shellvish commented Oct 14, 2023

Summary
Fixes functionality after invariant checks

Test plan

Dockernet test - main functionality

  1. Kick off RR changes by running make start-docker build=s && make test-integration-docker
  2. Set max inner bound $STRIDE_MAIN_CMD tx stakeibc set-redemption-rate-bounds GAIA 1 1.000001 --from $STRIDE_ADMIN_ACCT -y | TRIM_TX
  3. Observe zone is halted by querying it
➜  stride git:(inflation-fix) stridedl q stakeibc show-host-zone GAIA
host_zone:
  bech32prefix: cosmos
  chain_id: GAIA
...
  halted: true
  1. Verify IBC transfer off of Stride fails
# try to ibc tokens off stride
$STRIDE_MAIN_CMD tx ibc-transfer transfer transfer channel-0 cosmos1wv34rdclakwnex6zarphdznmfgysa0s7ewr7va2xjx4ncd339eys34w877 1stuatom --from val1 -y 
# check tx log
raw_log: 'failed to execute message; message index: 0: denom stuatom is blacklisted:
  1. Increase the max inner bound and unhalt the zone (1) $STRIDE_MAIN_CMD tx stakeibc set-redemption-rate-bounds GAIA 1 1.300001 --from $STRIDE_ADMIN_ACCT -y | TRIM_TX (2) $STRIDE_MAIN_CMD tx stakeibc resume-host-zone GAIA --from $STRIDE_ADMIN_ACCT -y | TRIM_TX
  2. Verify IBC transfer off of Stride works using same IBC transfer as above and checking tx logs
tx:
  '@type': /cosmos.tx.v1beta1.Tx
  auth_info:
    fee:
      amount: []
      gas_limit: "200000"
      granter: ""
      payer: ""
    signer_infos:
    - mode_info:
        single:
          mode: SIGN_MODE_DIRECT
      public_key:
        '@type': /cosmos.crypto.secp256k1.PubKey
        key: ApSI1puwW2K3iA4+pk9UnBzAlpjglmbSNmoXMG/8P1vt
      sequence: "7"
    tip: null
  body:
    extension_options: []
    memo: ""
    messages:
    - '@type': /ibc.applications.transfer.v1.MsgTransfer
      memo: ""
      receiver: cosmos1wv34rdclakwnex6zarphdznmfgysa0s7ewr7va2xjx4ncd339eys34w877
      sender: stride1uk4ze0x4nvh4fk0xm4jdud58eqn4yxhrt52vv7
      source_channel: channel-0
      source_port: transfer
      timeout_height:
        revision_height: "1446"
        revision_number: "0"
      timeout_timestamp: "1697316508458020000"
      token:
        amount: "1"
        denom: stuatom
    non_critical_extension_options: []
    timeout_height: "0"
  signatures:
  - JUegOL6rcWL8Of3N49kGKlQphxllrHalORBCI3GU+SRC5ES/YDGrC17ws/RFPa3iLpj5QFrE+kKVsb18LAtyOw==
txhash: D884711E6C7011AB858B5046ED354A435F40DA2131225C503
  1. Verify the host zone is not halted
➜  stride git:(inflation-fix) stridedl q stakeibc show-host-zone GAIA
host_zone:
  bech32prefix: cosmos
  chain_id: GAIA
...
  halted: false

Dockernet test - ledger

  1. Remove admin check in ValidateBasic
  2. Init a ledger account
  3. Call the function $STRIDE_MAIN_CMD tx stakeibc resume-host-zone GAIA --from ledger-test -y | TRIM_TX
  4. Check log
raw_log: 'failed to execute message; message index: 0: invalid chain id, zone for
  GAIA not halted: host zone is not halted'

(error indicates the signing happened correctly, which is what we care about when testing ledger)

ratelimitKeeper ratelimitkeeper.Keeper,
) upgradetypes.UpgradeHandler {
return func(ctx sdk.Context, _ upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) {
ctx.Logger().Info("Starting upgrade v15...")
Copy link
Contributor

@asalzmann asalzmann Oct 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ctx.Logger().Info("Starting upgrade v15...")
ctx.Logger().Info("Starting upgrade v16...")

sdk "github.com/cosmos/cosmos-sdk/types"
)

func (k msgServer) ResumeHostZone(goCtx context.Context, msg *types.MsgResumeHostZone) (*types.MsgResumeHostZoneResponse, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need to remove the blacklisted denom right?

@@ -26,6 +26,7 @@ func RegisterCodec(cdc *codec.LegacyAmino) {
cdc.RegisterConcrete(&MsgUndelegateHost{}, "stakeibc/UndelegateHost", nil)
cdc.RegisterConcrete(&MsgCalibrateDelegation{}, "stakeibc/CalibrateDelegation", nil)
cdc.RegisterConcrete(&MsgUpdateInnerRedemptionRateBounds{}, "stakeibc/UpdateInnerRedemptionRateBounds", nil)
cdc.RegisterConcrete(&MsgResumeHostZone{}, "stakeibc/ResumeHostZone", nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we test this with ledger?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a good point, let's test it in Dockernet

@@ -67,6 +67,9 @@ func NewMessageHandler(k keeper.Keeper) sdk.Handler {
case *types.MsgUpdateInnerRedemptionRateBounds:
res, err := msgServer.UpdateInnerRedemptionRateBounds(sdk.WrapSDKContext(ctx), msg)
return sdk.WrapServiceResult(ctx, res, err)
case *types.MsgResumeHostZone:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we test this on dockernet as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely

@shellvish shellvish merged commit 876810c into main Oct 14, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants