From e43edc47493ff8dd4cc6dff2ca556882dce065b9 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Thu, 15 Apr 2021 23:32:45 +0200 Subject: [PATCH 1/5] ADR-28 derive address functions (#9088) * update Derive functions * update adr-028 * changelog update * Apply suggestions from code review Co-authored-by: Marie Gauthier * review updates * remove DeriveMulti and rollback some changes in CHANGELOG * add noop error check Co-authored-by: Marie Gauthier --- CHANGELOG.md | 3 +++ .../adr-028-public-key-addresses.md | 24 +++++++++---------- types/address/hash.go | 16 +++++++++---- types/address/hash_test.go | 23 ++++++++++++++---- 4 files changed, 45 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a96d38f4bbd..4b591633a046 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [\#8786](https://github.com/cosmos/cosmos-sdk/pull/8786) Enabled secp256r1 in x/auth. * (rosetta) [\#8729](https://github.com/cosmos/cosmos-sdk/pull/8729) Data API fully supports balance tracking. Construction API can now construct any message supported by the application. * [\#8754](https://github.com/cosmos/cosmos-sdk/pull/8875) Added support for reverse iteration to pagination. +* [#9088](https://github.com/cosmos/cosmos-sdk/pull/9088) Added implementation to ADR-28 Derived Addresses. ### Client Breaking Changes * [\#8363](https://github.com/cosmos/cosmos-sdk/pull/8363) Addresses no longer have a fixed 20-byte length. From the SDK modules' point of view, any 1-255 bytes-long byte array is a valid address. @@ -82,6 +83,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/bank) [\#8798](https://github.com/cosmos/cosmos-sdk/pull/8798) `GetTotalSupply` is removed in favour of `GetPaginatedTotalSupply` * (x/bank/types) [\#9061](https://github.com/cosmos/cosmos-sdk/pull/9061) `AddressFromBalancesStore` now returns an error for invalid key instead of panic. + + ### State Machine Breaking * (x/{bank,distrib,gov,slashing,staking}) [\#8363](https://github.com/cosmos/cosmos-sdk/issues/8363) Store keys have been modified to allow for variable-length addresses. diff --git a/docs/architecture/adr-028-public-key-addresses.md b/docs/architecture/adr-028-public-key-addresses.md index 91c52e810d5e..d426e9969ff7 100644 --- a/docs/architecture/adr-028-public-key-addresses.md +++ b/docs/architecture/adr-028-public-key-addresses.md @@ -135,7 +135,7 @@ type Addressable interface { Address() []byte } -func NewComposed(typ string, subaccounts []Addressable) []byte { +func Composed(typ string, subaccounts []Addressable) []byte { addresses = map(subaccounts, \a -> LengthPrefix(a.Address())) addresses = sort(addresses) return address.Hash(typ, addresses[0] + ... + addresses[n]) @@ -175,7 +175,7 @@ func (multisig PubKey) Address() { prefix := fmt.Sprintf("%s/%d", proto.MessageName(multisig), multisig.Threshold) // use the Composed function defined above - return address.NewComposed(prefix, keys) + return address.Composed(prefix, keys) } ``` @@ -185,14 +185,14 @@ NOTE: this section is not finalize and it's in active discussion. In Basic Address section we defined a module account address as: -``` +```go address.Hash("module", moduleName) ``` We use `"module"` as a schema type for all module derived addresses. Module accounts can have sub accounts. The derivation process has a defined order: module name, submodule key, subsubmodule key. Module account addresses are heavily used in the SDK so it makes sense to optimize the derivation process: instead of using of using `LengthPrefix` for the module name, we use a null byte (`'\x00'`) as a separator. This works, because null byte is not a part of a valid module name. -``` +```go func Module(moduleName string, key []byte) []byte{ return Hash("module", []byte(moduleName) + 0 + key) } @@ -208,24 +208,24 @@ If we want to create an address for a module account depending on more than one btcAtomAMM := address.Module("amm", btc.Addrress() + atom.Address()}) ``` -We can continue the derivation process and can create an address for a submodule account. +#### Derived Addresses -``` -func Submodule(address []byte, derivationKey []byte) { - return Hash("module", address + derivationKey) +We must be able to cryptographically derive one address from another one. The derivation process must guarantee hash properties, hence we use the already defined `Hash` function: + +```go +func Derive(address []byte, derivationKey []byte) []byte { + return Hash(addres, derivationKey) } ``` -NOTE: if `address` is not a hash based address (with `LEN` length) then we should use `LengthPrefix`. An alternative would be to use one `Module` function, which takes a slice of keys and mapped with `LengthPrefix`. For final version we need to validate what's the most common use. - +Note: `Module` is a special case of the more general _derived_ address, where we set the `"module"` string for the _from address_. **Example** For a cosmwasm smart-contract address we could use the following construction: ``` -smartContractAddr := Submodule(Module("cosmwasm", smartContractsNamespace), smartContractKey) +smartContractAddr := Derived(Module("cosmwasm", smartContractsNamespace), []{smartContractKey}) ``` - ### Schema Types A `typ` parameter used in `Hash` function SHOULD be unique for each account type. diff --git a/types/address/hash.go b/types/address/hash.go index a6c9fe94c51c..8d9c5b39e11b 100644 --- a/types/address/hash.go +++ b/types/address/hash.go @@ -20,20 +20,21 @@ type Addressable interface { // Hash creates a new address from address type and key func Hash(typ string, key []byte) []byte { hasher := sha256.New() - hasher.Write(conv.UnsafeStrToBytes(typ)) + _, err := hasher.Write(conv.UnsafeStrToBytes(typ)) + // the error always nil, it's here only to satisfy the io.Writer interface + errors.AssertNil(err) th := hasher.Sum(nil) hasher.Reset() - _, err := hasher.Write(th) - // the error always nil, it's here only to satisfy the io.Writer interface + _, err = hasher.Write(th) errors.AssertNil(err) _, err = hasher.Write(key) errors.AssertNil(err) return hasher.Sum(nil) } -// NewComposed creates a new address based on sub addresses. -func NewComposed(typ string, subAddresses []Addressable) ([]byte, error) { +// Compose creates a new address based on sub addresses. +func Compose(typ string, subAddresses []Addressable) ([]byte, error) { as := make([][]byte, len(subAddresses)) totalLen := 0 var err error @@ -62,3 +63,8 @@ func Module(moduleName string, key []byte) []byte { mKey := append([]byte(moduleName), 0) return Hash("module", append(mKey, key...)) } + +// Derive derives a new address from the main `address` and a derivation `key`. +func Derive(address []byte, key []byte) []byte { + return Hash(conv.UnsafeBytesToStr(address), key) +} diff --git a/types/address/hash_test.go b/types/address/hash_test.go index be096c357fec..ae712c18dd4a 100644 --- a/types/address/hash_test.go +++ b/types/address/hash_test.go @@ -34,7 +34,7 @@ func (suite *AddressSuite) TestComposed() { a2 := addrMock{[]byte{21, 22}} typ := "multisig" - ac, err := NewComposed(typ, []Addressable{a1, a2}) + ac, err := Compose(typ, []Addressable{a1, a2}) assert.NoError(err) assert.Len(ac, Len) @@ -45,18 +45,18 @@ func (suite *AddressSuite) TestComposed() { assert.Equal(ac, ac2, "NewComposed works correctly") // changing order of addresses shouldn't impact a composed address - ac2, err = NewComposed(typ, []Addressable{a2, a1}) + ac2, err = Compose(typ, []Addressable{a2, a1}) assert.NoError(err) assert.Len(ac2, Len) assert.Equal(ac, ac2, "NewComposed is not sensitive for order") // changing a type should change composed address - ac2, err = NewComposed(typ+"other", []Addressable{a2, a1}) + ac2, err = Compose(typ+"other", []Addressable{a2, a1}) assert.NoError(err) assert.NotEqual(ac, ac2, "NewComposed must be sensitive to type") // changing order of addresses shouldn't impact a composed address - ac2, err = NewComposed(typ, []Addressable{a1, addrMock{make([]byte, 300, 300)}}) + ac2, err = Compose(typ, []Addressable{a1, addrMock{make([]byte, 300, 300)}}) assert.Error(err) assert.Contains(err.Error(), "should be max 255 bytes, got 300") } @@ -75,6 +75,21 @@ func (suite *AddressSuite) TestModule() { assert.NotEqual(addr2, addr3, "changing key must change address") } +func (suite *AddressSuite) TestDerive() { + assert := suite.Assert() + var addr, key1, key2 = []byte{1, 2}, []byte{3, 4}, []byte{1, 2} + d1 := Derive(addr, key1) + d2 := Derive(addr, key2) + d3 := Derive(key1, key2) + assert.Len(d1, Len) + assert.Len(d2, Len) + assert.Len(d3, Len) + + assert.NotEqual(d1, d2) + assert.NotEqual(d1, d3) + assert.NotEqual(d2, d3) +} + type addrMock struct { Addr []byte } From 0bcd7c6862f9bb8772082f54c53fcf936b984a8b Mon Sep 17 00:00:00 2001 From: Cyrus Goh Date: Fri, 16 Apr 2021 00:46:05 -0700 Subject: [PATCH 2/5] ci: check docs build (#9125) Co-authored-by: Alessio Treglia --- .github/workflows/check-docs.yml | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 .github/workflows/check-docs.yml diff --git a/.github/workflows/check-docs.yml b/.github/workflows/check-docs.yml new file mode 100644 index 000000000000..2954217d8c5e --- /dev/null +++ b/.github/workflows/check-docs.yml @@ -0,0 +1,25 @@ +name: Check docs build +# This workflow runs when a PR is labeled with `docs` +# This will check if the docs build successfully by running `npm run build` +on: + pull_request: + types: [ labeled ] + +jobs: + check-docs-build: + if: ${{ github.event.label.name == 'docs' }} + + name: Check docs build + runs-on: ubuntu-latest + steps: + - name: Checkout 🛎️ + uses: actions/checkout@v2.3.1 + with: + persist-credentials: false + fetch-depth: 0 + + - name: Install dependencies and build docs 🧱 + run: | + cd docs + npm install + npm run build From 89cb9b0f8c8326a6087397505baafa19b5ce9c06 Mon Sep 17 00:00:00 2001 From: Cyrus Goh Date: Fri, 16 Apr 2021 00:59:21 -0700 Subject: [PATCH 3/5] docs: uml hotfix (#9124) * comment out ungenerated uml .svg in the md files * generate svgs to docs/uml * create docs/uml/svg * move .punl to docs/uml/puml * update uml path in md files * docs: update relative path of uml svg in `x` md files * docs: remove plantuml script from pre.sh Co-authored-by: Marko --- docs/building-modules/msg-services.md | 2 +- docs/core/ocap.md | 2 +- docs/pre.sh | 2 - .../begin_redelegation_sequence.puml | 0 docs/uml/{ => puml}/delegation_sequence.puml | 0 docs/uml/{ => puml}/keeper_dependencies.puml | 0 docs/uml/{ => puml}/transaction_flow.puml | 2 - docs/uml/{ => puml}/unbond_sequence.puml | 0 docs/uml/svg/begin_redelegation_sequence.svg | 106 ++++++++++ docs/uml/svg/delegation_sequence.svg | 192 ++++++++++++++++++ docs/uml/svg/keeper_dependencies.svg | 102 ++++++++++ docs/uml/svg/transaction_flow.svg | 48 +++++ docs/uml/svg/unbond_sequence.svg | 110 ++++++++++ x/staking/spec/03_messages.md | 6 +- 14 files changed, 563 insertions(+), 9 deletions(-) rename docs/uml/{ => puml}/begin_redelegation_sequence.puml (100%) rename docs/uml/{ => puml}/delegation_sequence.puml (100%) rename docs/uml/{ => puml}/keeper_dependencies.puml (100%) rename docs/uml/{ => puml}/transaction_flow.puml (89%) rename docs/uml/{ => puml}/unbond_sequence.puml (100%) create mode 100644 docs/uml/svg/begin_redelegation_sequence.svg create mode 100644 docs/uml/svg/delegation_sequence.svg create mode 100644 docs/uml/svg/keeper_dependencies.svg create mode 100644 docs/uml/svg/transaction_flow.svg create mode 100644 docs/uml/svg/unbond_sequence.svg diff --git a/docs/building-modules/msg-services.md b/docs/building-modules/msg-services.md index 29927ae8b2c3..bab289b8c227 100644 --- a/docs/building-modules/msg-services.md +++ b/docs/building-modules/msg-services.md @@ -56,7 +56,7 @@ This method takes care of marshaling the `res` parameter to protobuf and attachi This diagram shows a typical structure of an `Msg` Service, and how the message propagates through the module. -![](../uml/transaction_flow.svg) +![Transaction flow](../uml/svg/transaction_flow.svg) ## Legacy Amino `Msg`s diff --git a/docs/core/ocap.md b/docs/core/ocap.md index 2264d46bff67..ff413cc7250f 100644 --- a/docs/core/ocap.md +++ b/docs/core/ocap.md @@ -72,7 +72,7 @@ gaia app. The following diagram shows the current dependencies between keepers. -![](../uml/keeper_dependencies.svg) +![Keeper dependencies](../uml/svg/keeper_dependencies.svg) ## Next {hide} diff --git a/docs/pre.sh b/docs/pre.sh index c52b14847398..46b1db77d4f0 100755 --- a/docs/pre.sh +++ b/docs/pre.sh @@ -10,5 +10,3 @@ for D in ../x/*; do done cat ../x/README.md | sed 's/\.\/x/\/modules/g' | sed 's/spec\/README.md//g' | sed 's/\.\.\/docs\/building-modules\/README\.md/\/building-modules\/intro\.html/g' > ./modules/README.md - -# plantuml -tsvg uml/*.puml diff --git a/docs/uml/begin_redelegation_sequence.puml b/docs/uml/puml/begin_redelegation_sequence.puml similarity index 100% rename from docs/uml/begin_redelegation_sequence.puml rename to docs/uml/puml/begin_redelegation_sequence.puml diff --git a/docs/uml/delegation_sequence.puml b/docs/uml/puml/delegation_sequence.puml similarity index 100% rename from docs/uml/delegation_sequence.puml rename to docs/uml/puml/delegation_sequence.puml diff --git a/docs/uml/keeper_dependencies.puml b/docs/uml/puml/keeper_dependencies.puml similarity index 100% rename from docs/uml/keeper_dependencies.puml rename to docs/uml/puml/keeper_dependencies.puml diff --git a/docs/uml/transaction_flow.puml b/docs/uml/puml/transaction_flow.puml similarity index 89% rename from docs/uml/transaction_flow.puml rename to docs/uml/puml/transaction_flow.puml index bf01f04f68b7..d9b6cbcf626a 100644 --- a/docs/uml/transaction_flow.puml +++ b/docs/uml/puml/transaction_flow.puml @@ -1,5 +1,3 @@ -What happens after a transaction is unmarshalled and is processed by the SDK? - @startuml 'https://plantuml.com/sequence-diagram diff --git a/docs/uml/unbond_sequence.puml b/docs/uml/puml/unbond_sequence.puml similarity index 100% rename from docs/uml/unbond_sequence.puml rename to docs/uml/puml/unbond_sequence.puml diff --git a/docs/uml/svg/begin_redelegation_sequence.svg b/docs/uml/svg/begin_redelegation_sequence.svg new file mode 100644 index 000000000000..5d17f311b928 --- /dev/null +++ b/docs/uml/svg/begin_redelegation_sequence.svg @@ -0,0 +1,106 @@ +RedelegationmsgServermsgServerkeeperkeeperstorestoreBeginRedelegation(delAddr, valSrcAddr, valDstAddr, sharesAmount)get number of sharewIf the delegator has more shares than the total shares in the validator(due to rounding errors), then just withdraw the max number of shares.check the redelegation uses correct denomalt[valSrcAddr == valDstAddr]erroralt[transitive redelegation]erroralt[already has max redelegations]errorthis is the number of redelegations for a specific (del, valSrc, valDst) tripledefault : 7Unbond(del, valSrc) returns returnAmountSee unbonding diagramalt[returnAmount is zero]errorDelegate(del, returnAmount, status := valSrc.status, valDst, subtractAccount := false)See delegation diagramalt[validator is unbonded]current timealt[unbonding not complete, or just started]create redelegation objectinsert redelegation in queue, to be processed at the appropriate timecompletion time of the redelegationemit event: delegator, valSrc, valSrc,sharesAmount, completionTime \ No newline at end of file diff --git a/docs/uml/svg/delegation_sequence.svg b/docs/uml/svg/delegation_sequence.svg new file mode 100644 index 000000000000..47ad53789dc4 --- /dev/null +++ b/docs/uml/svg/delegation_sequence.svg @@ -0,0 +1,192 @@ +Delegating (currently undelegated funds delegator)msgServer (staking)msgServer (staking)keeper (staking)keeper (staking)validatorvalidatorkeeper.bankKeeperkeeper.bankKeepervestingAccountvestingAccountctx.EventManagerctx.EventManagerstorestoreDelegate(Context, DelegatorAddress, Amount, Validator, tokenSrc := Unbonded)alt[exchange rate is invalid (tokens in validator is 0)]erroralt[perform a new delegation]delegation := create delegation objectBeforeDelegationCreated hookCalls IncrementValidatorPeriod (Used to calculate distribution) in keeper/validator.go[delegation exists, more tokens being added]BeforeDelegationModified hookwithdraw current delegation rewards (and increment period)alt[delegating from an account (subtractTokens == true)]DelegateCoinsFromAccountToModuleDelegateCoinsFromAccountToModule functionDelegateCoinsFromAccountToModuleDelegateCoinsDelegateCoins functionCheck the delegator has enough balances of all tokens delegatedTrack delegation (register that it exists to keep track of it)alt[validator is currently bonded]Transfer tokens from delegator to BondedTokensPool.[validator is currently unbonded or unbonding]Transfer tokens from delegator to NotBondedTokensPool.trackDelegation functiontrackDelegationalt[delegator is a vesting account]keep track of this delegationnil (success)[moving tokens between pools (subtractTokens == false)]alt[delegator tokens are not bonded but validator is bonded]SendCoinsFromModuleToModule(notBondedPool, bondedPool, coins)[delegator tokens are bonded but validator is not bonded]SendCoinsFromModuleToModule(bondedPool, notBondedPool, coins)SendCoins functionSendCoinsEmit TransferEvent(to, from, amount)alt[amount of spendable (balance - locked) coins too low]errorsubtract balance from senderadd balance to recipientAddTokensFromDelcalculate number of shares to issueIf there are no shares (validator being created) then 1 token = 1 share.If there are already shares, thenadded shares = (added tokens amount) * (current validator shares) / (current validator tokens)add delegated tokens to validatorvalidator, addedSharesupdate validator statecalculate new validator's powerNumber of tokens divided by PowerReduction (default: 1,000,000,000,000,000,000 = 10^18)alt[validator is not jailed]update validator's power in power indexthe power index has entries shaped as 35 || power || address.This makes the validators sorted by power, high to low.AfterDelegationModified hookCalls initializeDelegationStore the previous periodCalculate the number of tokens from shares(shares the delegator has) * (tokens in delegation object)/(total tokens delegated to the validator)Store delegation starting info.newShares (ignored by Delegate function)Emit event: Delegation(ValidatorAddress)Emit event: Message(DelegatorAddress)telemetry(Amount, Denom) \ No newline at end of file diff --git a/docs/uml/svg/keeper_dependencies.svg b/docs/uml/svg/keeper_dependencies.svg new file mode 100644 index 000000000000..bac9328e3d37 --- /dev/null +++ b/docs/uml/svg/keeper_dependencies.svg @@ -0,0 +1,102 @@ +The dependencies between Keepers (Feb 2021)StakingDistributionSlashingEvidenceBankAuth/AccountGovMint \ No newline at end of file diff --git a/docs/uml/svg/transaction_flow.svg b/docs/uml/svg/transaction_flow.svg new file mode 100644 index 000000000000..1ae962de344a --- /dev/null +++ b/docs/uml/svg/transaction_flow.svg @@ -0,0 +1,48 @@ +UserUserbaseAppbaseApprouterrouterhandlerhandlermsgServermsgServerkeeperkeeperContext.EventManagerContext.EventManagerTransaction Type<Tx>Route(ctx, msgRoute)handlerMsg<Tx>(Context, Msg(...))<Tx>(Context, Msg)alt[addresses invalid, denominations wrong, etc.]errorperform action, update contextresults, error codeEmit relevant eventsmaybe wrap results in more structureresult, error coderesults, error code \ No newline at end of file diff --git a/docs/uml/svg/unbond_sequence.svg b/docs/uml/svg/unbond_sequence.svg new file mode 100644 index 000000000000..7219f200a5bf --- /dev/null +++ b/docs/uml/svg/unbond_sequence.svg @@ -0,0 +1,110 @@ +UndelegatemsgServermsgServerkeeperkeeperstorestorebankKeeperbankKeeperUndelegate(delAddr, valAddr, tokenAmount)calculate number of shares the tokenAmount representsalt[wrong denom]errorUnbond(delAddr, valAddr, shares)BeforeDelegationSharesModified hookalt[no such delegation]erroralt[not enough shares]erroralt[delegator is the operator of the validatorand validator is not already jailedand unbonding would put self-delegation under min threshold]jail the validator, but proceed with unbondingDefault min delegation threshold : 1 sharealt[complete unbonding, all shares removed]remove delegation object[there are still shares delegated (not a complete undbonding)]update delegation objectAfterDelegationModified hookupdate validator power indexupdate validator information (including token amount)alt[validator status is "unbonded" and it has no more tokens]delete the validatorotherwise, do this in EndBlock once validator is unbondedalt[validator is bonded]send tokens from bonded pool to not bonded poolemit event : EventTypeUnbond(delAddr, valAddr, tokenAmount, completion time) \ No newline at end of file diff --git a/x/staking/spec/03_messages.md b/x/staking/spec/03_messages.md index b6fdec6f7803..8b5fa28d6698 100644 --- a/x/staking/spec/03_messages.md +++ b/x/staking/spec/03_messages.md @@ -80,7 +80,7 @@ tracked in validator object in the `Validators` index. It is possible to delegate to a jailed validator, the only difference being it will not be added to the power index until it is unjailed. -![](docs/uml/delegation_sequence.svg) +![Delegation sequence](../../../docs/uml/svg/delegation_sequence.svg) ## Msg/Undelegate @@ -114,7 +114,7 @@ When this service message is processed the following actions occur: - if there are no more `Shares` in the delegation, then the delegation object is removed from the store - under this situation if the delegation is the validator's self-delegation then also jail the validator. -![](docs/uml/unbond_sequence.svg) +![Unbond sequence](../../../docs/uml/svg/unbond_sequence.svg) ## Msg/BeginRedelegate @@ -151,4 +151,4 @@ When this service message is processed the following actions occur: - if there are no more `Shares` in the source delegation, then the source delegation object is removed from the store - under this situation if the delegation is the validator's self-delegation then also jail the validator. -![](docs/uml/begin_redelegation_sequence.svg) +![Begin redelegation sequence](../../../docs/uml/svg/begin_redelegation_sequence.svg) From 82dc0081fdc06fa6cf8bd6a0d87987d27ac7b1ca Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Fri, 16 Apr 2021 03:43:49 -0500 Subject: [PATCH 4/5] Minor docs update to sdk.Int (#9126) Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> --- types/int.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/types/int.go b/types/int.go index e8fada2e78ea..1a013bc0549e 100644 --- a/types/int.go +++ b/types/int.go @@ -69,9 +69,9 @@ func unmarshalText(i *big.Int, text string) error { var _ CustomProtobufType = (*Int)(nil) -// Int wraps integer with 256 bit range bound +// Int wraps big.Int with a 256 bit range bound // Checks overflow, underflow and division by zero -// Exists in range from -(2^maxBitLen-1) to 2^maxBitLen-1 +// Exists in range from -(2^255 - 1) to 2^255 - 1 type Int struct { i *big.Int } From b4fc48ca71e99f25382e5b9aa014ff1189988ee6 Mon Sep 17 00:00:00 2001 From: Amaury <1293565+amaurym@users.noreply.github.com> Date: Sat, 17 Apr 2021 00:00:08 +0200 Subject: [PATCH 5/5] Add comment on x/auth hacky migration script (#9132) * Add comment on x/auth hacky migration script * QueryServer -> QueryRouter --- x/auth/legacy/v043/store.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/x/auth/legacy/v043/store.go b/x/auth/legacy/v043/store.go index 27db787f846b..f95f1b3a1bb7 100644 --- a/x/auth/legacy/v043/store.go +++ b/x/auth/legacy/v043/store.go @@ -1,3 +1,20 @@ +// Package v043 creates in-place store migrations for fixing tracking +// delegations with vesting accounts. +// ref: https://github.com/cosmos/cosmos-sdk/issues/8601 +// ref: https://github.com/cosmos/cosmos-sdk/issues/8812 +// +// The migration script modifies x/auth state, hence lives in the `x/auth/legacy` +// folder. However, it needs access to staking and bank state. To avoid +// cyclic dependencies, we cannot import those 2 keepers in this file. To solve +// this, we use the baseapp router to do inter-module querying, by importing +// the `baseapp.QueryRouter grpc.Server`. This is really hacky. +// +// PLEASE DO NOT REPLICATE THIS PATTERN IN YOUR OWN APP. +// +// Proposals to refactor this file have been made in: +// https://github.com/cosmos/cosmos-sdk/issues/9070 +// The preferred solution is to use inter-module communication (ADR-033), and +// this file will be refactored to use ADR-033 once it's ready. package v043 import ( @@ -27,6 +44,8 @@ const ( balancesPath = "/cosmos.bank.v1beta1.Query/AllBalances" ) +// We use the baseapp.QueryRouter here to do inter-module state querying. +// PLEASE DO NOT REPLICATE THIS PATTERN IN YOUR OWN APP. func migrateVestingAccounts(ctx sdk.Context, account types.AccountI, queryServer grpc.Server) (types.AccountI, error) { bondDenom, err := getBondDenom(ctx, queryServer) @@ -111,6 +130,8 @@ func resetVestingDelegatedBalances(evacct exported.VestingAccount) (exported.Ves } } +// We use the baseapp.QueryRouter here to do inter-module state querying. +// PLEASE DO NOT REPLICATE THIS PATTERN IN YOUR OWN APP. func getDelegatorDelegationsSum(ctx sdk.Context, address string, queryServer grpc.Server) (sdk.Coins, error) { querier, ok := queryServer.(*baseapp.GRPCQueryRouter) if !ok { @@ -153,6 +174,8 @@ func getDelegatorDelegationsSum(ctx sdk.Context, address string, queryServer grp return res, nil } +// We use the baseapp.QueryRouter here to do inter-module state querying. +// PLEASE DO NOT REPLICATE THIS PATTERN IN YOUR OWN APP. func getDelegatorUnbondingDelegationsSum(ctx sdk.Context, address, bondDenom string, queryServer grpc.Server) (sdk.Coins, error) { querier, ok := queryServer.(*baseapp.GRPCQueryRouter) if !ok { @@ -197,6 +220,8 @@ func getDelegatorUnbondingDelegationsSum(ctx sdk.Context, address, bondDenom str return res, nil } +// We use the baseapp.QueryRouter here to do inter-module state querying. +// PLEASE DO NOT REPLICATE THIS PATTERN IN YOUR OWN APP. func getBalance(ctx sdk.Context, address string, queryServer grpc.Server) (sdk.Coins, error) { querier, ok := queryServer.(*baseapp.GRPCQueryRouter) if !ok { @@ -229,6 +254,8 @@ func getBalance(ctx sdk.Context, address string, queryServer grpc.Server) (sdk.C return balance.Balances, nil } +// We use the baseapp.QueryRouter here to do inter-module state querying. +// PLEASE DO NOT REPLICATE THIS PATTERN IN YOUR OWN APP. func getBondDenom(ctx sdk.Context, queryServer grpc.Server) (string, error) { querier, ok := queryServer.(*baseapp.GRPCQueryRouter) if !ok { @@ -264,6 +291,9 @@ func getBondDenom(ctx sdk.Context, queryServer grpc.Server) (string, error) { // MigrateAccount migrates vesting account to make the DelegatedVesting and DelegatedFree fields correctly // track delegations. // References: https://github.com/cosmos/cosmos-sdk/issues/8601, https://github.com/cosmos/cosmos-sdk/issues/8812 +// +// We use the baseapp.QueryRouter here to do inter-module state querying. +// PLEASE DO NOT REPLICATE THIS PATTERN IN YOUR OWN APP. func MigrateAccount(ctx sdk.Context, account types.AccountI, queryServer grpc.Server) (types.AccountI, error) { return migrateVestingAccounts(ctx, account, queryServer) }