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

[v16] SDK v47 upgrade #646

Merged
merged 140 commits into from
May 22, 2023
Merged

[v16] SDK v47 upgrade #646

merged 140 commits into from
May 22, 2023

Conversation

faddat
Copy link
Contributor

@faddat faddat commented Apr 24, 2023

Closes #644

Style notes:

This PR moves all external modules into Juno. To understand why this is being done, allow me to share the sdk-v47 upgrade issue for interchain security:

So, the trouble is that we can't depend on Gaia updating to 47 in a timely manner. Likewise on the new token-factory repository, there wasn't an sdk branch. I think that it is much cleaner to have the needed code in Juno, instead of waiting on upstreams that may be unreliable or slow to upgrade.

Since juno does not use a forked sdk, chain teams wishing to use Juno's very nice (thanks @Reecepbcups) token factory work, can just use a version of juno that matches their desired cosmos-sdk version.

Then, we get into ibc-hooks. Same story. Osmosis ibc-hooks work is great, but does not support sdk 47 or ibc 7. We need that. In nursery, I am handling that by adding ibc-hooks to nursery.

There needs to be a place where this work is integrated and kept up to date. For strategic reasons, I am suggesting that Juno would be a great place for that to happen.

An alternative that I can see is to do this in github.com/notional-labs/nursery, but then, it does not give juno control, if the whole notional team is hit by a bus, juno has to do this regardless.

Juno should not be dependent on others ship schedules, even notional's and especially gaia's.

Description

  • add ibc-hooks to juno's repository based on osmosis main branch
  • add token-factory to juno's repository based on CosmosTokenFactory main branch
  • add globalfee to juno's repository based on gaia's main branch

Subtasks

  • change to the new style for reducing the block time
  • get consensus around where modules should sit, if they sit outside of here, we should upstream changes to the other repositories
  • inform Dang that he can just make PR's to the 47-upgrade branch

@Reecepbcups
Copy link
Contributor

I'm probably a strong no with veto on this.

Going to sleep on it

proto/Dockerfile Outdated Show resolved Hide resolved
proto/buf.yaml Outdated Show resolved Hide resolved
@faddat
Copy link
Contributor Author

faddat commented May 19, 2023

Just so you know, I'm unable to review this PR because it's my own PR, I'm going to do a quick check over everything, and I want to thank you for your work on it

@faddat
Copy link
Contributor Author

faddat commented May 19, 2023

@Reecepbcups nice catch. Will have a q for you in tg, on my editor I am getting an "ambiguous import error"

Screenshot 2023-05-20 at 2 15 12 AM

I've also pinned the versions for the buf libraries.

Reecepbcups and others added 2 commits May 19, 2023 20:59
Use upstream test features + Enable IBCFee for all transfer channels
@dimiandre
Copy link
Member

Above commission rate change (from ante to staking keeper) tested ^^

raw_log: 'failed to execute message; message index: 0: cannot set validator commission to less than minimum rate of 0.050000000000000000: commission cannot be less than min rate'

Then 0.05 commission worked.

(also added it to the v15 upgrade handler)

have you tried also with Authz?

@Reecepbcups
Copy link
Contributor

@dimiandre Yes, it fails since the underlying message itself fails. The Authz check only had to happen with our impl. since we had to check all subMessages to throw out (Because the subMsg/msg create itself was not being checked by the SDK)

Further proof just to double check though

# junod keys list --keyring-backend test --home $HOME/.juno1

export JUNOD_NODE=http://localhost:26657

# grant juno1 to make a val on FeeAcc's behalf
junod tx authz grant juno1hj5fveer5cjtn4wd6wstzugjfdxzl0xps73ftl generic --msg-type=/cosmos.staking.v1beta1.MsgCreateValidator --from feeacc --keyring-backend test --home $HOME/.juno1 --chain-id local-1 --fees 500ujuno --yes

junod tx authz exec <(junod tx staking create-validator --moniker=new_val --commission-rate=0.01 --commission-max-rate=1.0 --commission-max-change-rate=0.2 --from=feeacc --min-self-delegation=1 --amount 100ujuno --pubkey=$(junod keys show feeacc --keyring-backend=test --home=$HOME/.juno1 --pubkey) --home $HOME/.juno1 --keyring-backend=test --chain-id=local-1 --generate-only) --from juno1 --home=$HOME/.juno1 --chain-id=local-1 --keyring-backend=test --yes --fees=500ujuno


raw_log: 'failed to execute message; message index: 0: failed to execute message;
  message description:<moniker:"new_val" > commission:<rate:"10000000000000000" max_rate:"1000000000000000000"
  max_change_rate:"200000000000000000" > min_self_delegation:"1" delegator_address:"juno1efd63aw40lxf3n4mhf7dzhjkr453axurv2zdzk"
  validator_address:"junovaloper1efd63aw40lxf3n4mhf7dzhjkr453axurnh5ze0" pubkey:<type_url:"/cosmos.crypto.secp256k1.PubKey"
  value:"\n!\002\345,\341H\357\220\234K\206\273\366\346\254j(\242Y\256Cx\\\022\350\356\325\321dcDxFJ"
  > value:<denom:"ujuno" amount:"100" > : cannot set validator commission to less
  than minimum rate of 0.050000000000000000: commission cannot be less than min rate'

@Reecepbcups
Copy link
Contributor

protoVer=0.11.6 to 13

@faddat
Copy link
Contributor Author

faddat commented May 22, 2023

bump cometbft-db to v0.8.0

Copy link
Contributor

@JakeHartnell JakeHartnell left a comment

Choose a reason for hiding this comment

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

LFG! 🚀

Thanks for the wonderful walk through on the call today!

Copy link
Contributor

@JakeHartnell JakeHartnell left a comment

Choose a reason for hiding this comment

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

LFG! 🚀

Thanks for the wonderful walk through on the call today!

@Reecepbcups Reecepbcups merged commit aab43aa into main May 22, 2023
@Reecepbcups Reecepbcups removed the review me Review me for merge label May 22, 2023
@Reecepbcups Reecepbcups changed the title SDK v47 upgrade [v16] SDK v47 upgrade May 24, 2023
@joelsmith-2019 joelsmith-2019 deleted the 47-upgrade branch February 2, 2024 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sdk 47
4 participants