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

added stToken burn to unregister host zone function #1175

Merged
merged 5 commits into from
Apr 13, 2024
Merged

Conversation

sampocs
Copy link
Collaborator

@sampocs sampocs commented Apr 12, 2024

Context

See PR #1166 for context on the unregister host zone function. TLDR is it's only for our testnet.

This PR extends the function to also remove redemption records and burn any outstanding stTokens.

Testing

  • Updated RegisterHostZone to "unregister" if the host zone exists. This allows me to test the function by first running the register tx back to back (with the first execution adding it, and the second one removing it)
	// get zone
	_, found = k.GetHostZone(ctx, chainId)
	if found {
		if err := k.UnregisterHostZone(ctx, chainId); err != nil {
			return nil, err
		}
		return &types.MsgRegisterHostZoneResponse{}, nil
  • Started dockernet and confirmed the host zone was registered
  • Liquid staked and transferred tokens off stride
gaiadl tx ibc-transfer transfer transfer channel-0 stride1uk4ze0x4nvh4fk0xm4jdud58eqn4yxhrt52vv7 10000uatom --from gval1 -y
stridedl tx stakeibc liquid-stake 10000 uatom --from val1 -y
stridedl tx ibc-transfer transfer transfer channel-0 cosmos1uk4ze0x4nvh4fk0xm4jdud58eqn4yxhrgl2scj 1000stuatom --from val1 -y
  • Confirmed stToken balances and supply before the host is unregistered
stridedl q bank balances $(stridedl keys show val1 -a) --denom stuatom
stridedl q bank balances $(stridedl q ibc-transfer escrow-address transfer channel-0) --denom stuatom
stridedl q bank total --denom=stuatom
stridedl q ibc-transfer total-escrow stuatom
  • Call the register host command which will unregister
stridedl tx stakeibc register-host-zone \
    connection-0 uatom cosmos \
    ibc/27394FB092D2ECCD56123C74F36E4C1F926001CEADA9CA97EA622B25F41E5EB2 \
    channel-0 1 true \
    --from admin -y --gas 4000000
  • Confirmed all balances were set back to 0
stridedl q bank balances $(stridedl keys show val1 -a) --denom stuatom
stridedl q bank balances $(stridedl q ibc-transfer escrow-address transfer channel-0) --denom stuatom
stridedl q bank total --denom=stuatom
stridedl q ibc-transfer total-escrow stuatom
  • Exec into the relayer and create a new channel
rly tx link stride-gaia --override
  • Run the registration command again to re-register with the new connection ID and channel ID
stridedl tx stakeibc register-host-zone \
    connection-1 uatom cosmos \
    ibc/25418646C017D377ADF3202FF1E43590D0DAE3346E594E8D78176A139A928F88 \
    channel-7 1 true \
    --from admin -y --gas 4000000
  • Liquid stake again and confirm the delegation goes through and there's no chain halt

Copy link
Contributor

@ethan-stride ethan-stride left a comment

Choose a reason for hiding this comment

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

LGTM
Does this work for the other hostZones like stTia and stDym which are non-standard? The ones with operator accounts instead of ICAs?

@sampocs
Copy link
Collaborator Author

sampocs commented Apr 12, 2024

it does not cause those are in a different module

@ethan-stride
Copy link
Contributor

it does not cause those are in a different module

Sorry yes, I knew that this currently wouldn't affect those. I should have asked don't we need this same type of logic for resetting those ones in the other module and specifically will this accounts keeper method to burn all the stTokens work with the operator accounts on the testnet the same way?

@sampocs
Copy link
Collaborator Author

sampocs commented Apr 12, 2024

oh I see - good question! i don't think we do atm. Hopefully those host zones become normal ICA host zones by the time we ever have to use this again. If not, we could always make the custom version when needed

@sampocs sampocs merged commit bdef5af into main Apr 13, 2024
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.

2 participants