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

Make Stride a consumer chain #811

Merged
merged 95 commits into from
Jul 15, 2023
Merged

Make Stride a consumer chain #811

merged 95 commits into from
Jul 15, 2023

Conversation

antstalepresh
Copy link
Contributor

@antstalepresh antstalepresh commented Jun 1, 2023

Context and purpose of the change

This consumer branch uses informal's ICS version for consumer chain.

How to test ICS with s->c transition

- ICS setup

  1. Checkout ics_docker_infra (sdk47) branch and build a binary from main branch using following command.
make UPGRADE_OLD_VERSION=main upgrade-build-old-binary
  1. Update submodules and run dockernet
make UPGRADE_OLD_VERSION=main UPGRADE_NAME=v12 start-docker -sghr
  1. Once stride,gaia, and hermes start running, run upgrade process using following command.
make UPGRADE_NAME=v12 submit-upgrade-immediately
  1. After submitting the proposal, start ICS setup using following command.
make setup-ics

- Test VP sharing between consumer and provider

Once new ICS channel is established, try unbonding on gaia.

gaiadl tx staking unbond cosmosvaloper1uk4ze0x4nvh4fk0xm4jdud58eqn4yxhrdt795p 32000000uatom --from gval1 --broadcast-mode block -y --gas auto

And then, check VP on both stride and gaia.

gaiadl status
stridedl status

You should have same VP for both gaia and stride nodes.

- Test reward distribution to provider validators

Some portion (currently 25%) of total rewards from stride chain are distributed to provider validators every 1k blocks.
Check if the validator rewards include stride IBC tokens on gaia after passing 1k blocks.

gaiadl query distribution validator-outstanding-rewards cosmosvaloper1uk4ze0x4nvh4fk0xm4jdud58eqn4yxhrdt795p

- Test staking rewards for stride governors

Remaining rewards (75%) should be distributed to stride governors and community pool on stride.
Check if the governor rewards increase continuously properly.

stridedl query distribution validator-outstanding-rewards stridevaloper1uk4ze0x4nvh4fk0xm4jdud58eqn4yxhrgpwsqm

- Test staking/unstaking to Stride governors

Staking/Unstaking to governors should work as normal.
Check if tokens are bonded to governors and unbonded tokens are sent to delegators properly after unbonding period.

stridedl tx staking delegate stridevaloper1nnurja9zt97huqvsfuartetyjx63tc5zrj5x9f 150ustrd --from val1 -y
stridedl tx staking unbond stridevaloper1nnurja9zt97huqvsfuartetyjx63tc5zrj5x9f 150ustrd --from val1 -y

stana-miric and others added 30 commits November 24, 2022 10:34
Co-authored by:
Dusan Maksimovic
Stana Miric
Co-authored-by: Son Trinh <sontrinh@Sons-MacBook-Pro.local>
Co-authored-by: Jacob Gadikian <jacobgadikian@gmail.com>
@github-actions github-actions bot removed the C:mint label Jun 27, 2023
Copy link
Contributor

@asalzmann asalzmann left a comment

Choose a reason for hiding this comment

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

Looks great overall!

Left some questions throughout.

@sampocs @strbrian we should think about how we want to run dockernet going forward - should Stride always launch as an ICS chain on dockernet?

.gitmodules Show resolved Hide resolved
x/stakeibc/keeper/keeper.go Outdated Show resolved Hide resolved
dockernet/upgrades/submit_upgrade.sh Outdated Show resolved Hide resolved
x/stakeibc/keeper/get_denom_traces_test.go Show resolved Hide resolved
testing/interchain_security_test.go Outdated Show resolved Hide resolved
app/upgrades/v12/upgrades.go Outdated Show resolved Hide resolved
app/apptesting/test_helpers.go Show resolved Hide resolved
app/app.go Outdated Show resolved Hide resolved
@@ -6,12 +6,14 @@ import (
"path/filepath"

authsims "github.com/cosmos/cosmos-sdk/x/auth/simulation"
"github.com/cosmos/cosmos-sdk/x/genutil"
Copy link
Contributor

Choose a reason for hiding this comment

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

@shellvish if you have time, might make sense for you to review this file as well!

app/app.go Show resolved Hide resolved
@asalzmann asalzmann changed the title Consumer sdk47 informal Make Stride a consumer chain Jul 14, 2023
@strbrian
Copy link
Contributor

should Stride always launch as an ICS chain on dockernet?

For now yes but there are ways running consumer without running provider for testing purpose.
Will look into that shortly.

Copy link
Contributor

@asalzmann asalzmann left a comment

Choose a reason for hiding this comment

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

resolution for #811 (comment)

{MsgType: stakeibctypes.ModuleName, Key: string(stakeibctypes.KeyRedemptionRateInterval)}: {},
{MsgType: stakeibctypes.ModuleName, Key: string(stakeibctypes.KeyStrideCommission)}: {},
{MsgType: stakeibctypes.ModuleName, Key: string(stakeibctypes.KeyReinvestInterval)}: {},
// {MsgType: stakeibctypes.ModuleName, Key: string(stakeibctypes.KeyValidatorRebalancingThreshold)}: {},
Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove this

{MsgType: stakeibctypes.ModuleName, Key: string(stakeibctypes.KeyIbcTimeoutBlocks)}: {},
{MsgType: stakeibctypes.ModuleName, Key: string(stakeibctypes.KeyFeeTransferTimeoutNanos)}: {},
{MsgType: stakeibctypes.ModuleName, Key: string(stakeibctypes.KeyMaxStakeICACallsPerEpoch)}: {},
// {MsgType: stakeibctypes.ModuleName, Key: string(stakeibctypes.KeySafetyMinRedemptionRateThreshold)}: {},
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to update the variable names that @strbrian sent in slack

{MsgType: stakingtypes.ModuleName, Key: string(stakingtypes.KeyBondDenom)}: {},
//distribution
{MsgType: distrtypes.ModuleName, Key: string(distrtypes.ParamStoreKeyCommunityTax)}: {},
// {MsgType: distrtypes.ModuleName, Key: string(distrtypes.ParamStoreKeyBaseProposerReward)}: {},
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove these

@@ -18,6 +20,19 @@ import (
stakeibctypes "github.com/Stride-Labs/stride/v12/x/stakeibc/types"
)

var WhiteListModule = map[string]struct{}{
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain this list / where did it come from? Does this restrict proposals to param changes?

Do we also need to whitelist software upgrade proposals in the gov module?

Copy link
Contributor

@strbrian strbrian Jul 14, 2023

Choose a reason for hiding this comment

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

Good question!
That came from here.
https://github.com/cosmos/interchain-security/blob/main/app/consumer-democracy/proposals_whitelisting.go#L41-L52C1

That doesn't restrict proposal change proposals at all.
IsWhiteListModule is only used for whitelisting non-legacy proposals.
For soft-upgrade proposal, if we submit as legacy proposal, we don't need to whitelist software upgrade proposal in WhitelistModule.
But if we submit as non-legacy proposal, the software upgrade proposal must be whitelisted in WhitelistModule.
To enable both cases, I will add cancel/upgrade proposal types to WhitelistModule.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding this. Will follow up with Informal to ask what set of permissions they recommend.

@@ -41,7 +40,6 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
ante.NewSetUpContextDecorator(),
ante.NewExtensionOptionsDecorator(options.ExtensionOptionChecker),
// temporarily disabled so that chain can be tested locally without the provider chain running
consumerante.NewMsgFilterDecorator(options.ConsumerKeeper),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this decorator unnecessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

It was originally disabled but enabling it blocked some IBC msgs while running integration tests(redemption).
So I removed it.

 ✗ [INTEGRATION-BASIC-GAIA] redemption works
   (from function `WAIT_FOR_STRING' in file dockernet/tests/integration_tests.bats, line 0,
    in test file dockernet/tests/integration_tests.bats, line 210)
     `WAIT_FOR_STRING $STRIDE_LOGS "\[REDEMPTION] completed on $HOST_CHAIN_ID"' failed with status 130
   code: 1
   codespace: undefined
   data: ""
   events: []
   gas_used: "0"
   gas_wanted: "0"
   height: "0"
   info: ""
   logs: []
   raw_log: tx contains unsupported message types at height 353
   timestamp: ""
   tx: null
   txhash: 9783021376368CC3B5AD692940B006446B4E781A1446DD4DCCACACC277539115

   Received SIGINT, aborting ...

   bats warning: Executed 6 instead of expected 9 tests

@asalzmann asalzmann marked this pull request as ready for review July 14, 2023 16:32
@asalzmann asalzmann merged commit b4abd61 into main Jul 15, 2023
asalzmann added a commit that referenced this pull request Jul 15, 2023
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.

7 participants