From d2cc0509199487b798bf9e3b219edbac01d02cc1 Mon Sep 17 00:00:00 2001 From: marcello33 Date: Mon, 29 Jan 2024 10:54:40 +0100 Subject: [PATCH] chg: address PR comments / temp comment out scheduled CI jobs --- .github/dependabot.yml | 508 +++++++++---------- .github/workflows/clean-action-artifacts.yml | 34 +- .github/workflows/sims-045.yml | 4 +- .github/workflows/sims-046.yml | 4 +- .github/workflows/sims-047.yml | 4 +- .github/workflows/sims-nightly.yml | 4 +- .github/workflows/sims.yml | 4 +- .github/workflows/stale.yml | 52 +- client/keys/show_test.go | 35 +- types/address.go | 8 +- types/address_test.go | 70 ++- types/bech32/legacybech32/pk.go | 4 +- types/errors/errors.go | 6 + x/auth/ante/fee.go | 34 +- x/auth/ante/feegrant_test.go | 65 +++ x/auth/ante/setup.go | 6 + x/auth/ante/sigverify.go | 9 - x/auth/ante/validator_tx_fee.go | 2 +- x/auth/keeper/keeper.go | 23 +- x/auth/keeper/keeper_test.go | 31 ++ x/auth/types/account.go | 5 +- x/auth/types/params.go | 7 +- x/group/keeper/msg_server.go | 16 +- x/group/migrations/v2/gen_state.go | 19 +- x/group/migrations/v2/migrate.go | 21 +- 25 files changed, 579 insertions(+), 396 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index c29e3542ca59..cf444e55e647 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,254 +1,254 @@ -# Please see the documentation for all configuration options: -# https://help.github.com/github/administering-a-repository/configuration-options-for-dependency-updates - -version: 2 -updates: - - package-ecosystem: github-actions - directory: "/" - schedule: - interval: daily - time: "01:00" - - - package-ecosystem: npm - directory: "/docs" - schedule: - interval: weekly - # DevRel should review docs updates - assignees: - - "julienrbrt" - - - package-ecosystem: gomod - directory: "/" - schedule: - interval: daily - time: "01:05" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "/simapp" - schedule: - interval: daily - time: "01:10" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "/tests" - schedule: - interval: weekly - day: monday - time: "01:15" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "/api" - schedule: - interval: weekly - day: tuesday - time: "01:20" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "/orm" - schedule: - interval: weekly - day: wednesday - time: "01:25" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "/core" - schedule: - interval: weekly - day: thursday - time: "01:30" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "/depinject" - schedule: - interval: weekly - day: friday - time: "01:35" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "/errors" - schedule: - interval: weekly - day: monday - time: "01:40" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "/math" - schedule: - interval: weekly - day: tuesday - time: "01:45" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "/client/v2" - schedule: - interval: weekly - day: wednesday - time: "01:50" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "/x/tx" - schedule: - interval: weekly - day: thursday - time: "01:55" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "/tools/cosmovisor" - schedule: - interval: weekly - day: friday - time: "02:00" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "/tools/confix" - schedule: - interval: weekly - day: tuesday - time: "02:10" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "/tools/hubl" - schedule: - interval: weekly - day: thursday - time: "02:15" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "/collections" - schedule: - interval: weekly - day: friday - time: "02:20" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "/x/nft" - schedule: - interval: weekly - day: monday - time: "02:25" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "/x/circuit" - schedule: - interval: weekly - day: tuesday - time: "02:30" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "x/feegrant" - schedule: - interval: weekly - day: wednesday - time: "02:35" - labels: - - "A:automerge" - - dependencies - - - package-ecosystem: gomod - directory: "/x/evidence" - schedule: - interval: weekly - day: thursday - time: "02:40" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "/store" - schedule: - interval: weekly - day: friday - time: "02:45" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "x/upgrade" - schedule: - interval: weekly - day: monday - time: "02:50" - labels: - - "A:automerge" - - dependencies - - package-ecosystem: gomod - directory: "log" - schedule: - interval: weekly - day: tuesday - time: "02:55" - labels: - - "A:automerge" - - dependencies - - # Dependencies should be up to date on release branch - - package-ecosystem: gomod - directory: "/" - target-branch: "release/v0.47.x" - schedule: - interval: daily - time: "03:00" - labels: - - "A:automerge" - - dependencies - - "testing-required" - allow: - - dependency-name: "github.com/cosmos/cosmos-sdk/*" - dependency-type: "all" - - dependency-name: "github.com/cosmos/*" - dependency-type: "all" - - dependency-name: "cosmossdk.io/*" - dependency-type: "all" - - # Dependencies should be up to date on release branch - - package-ecosystem: gomod - directory: "/" - target-branch: "release/v0.50.x" - schedule: - interval: daily - time: "03:00" - labels: - - "A:automerge" - - dependencies - - "testing-required" - allow: - - dependency-name: "github.com/cosmos/cosmos-sdk/*" - dependency-type: "all" - - dependency-name: "github.com/cosmos/*" - dependency-type: "all" - - dependency-name: "cosmossdk.io/*" - dependency-type: "all" +## Please see the documentation for all configuration options: +## https://help.github.com/github/administering-a-repository/configuration-options-for-dependency-updates +# +#version: 2 +#updates: +# - package-ecosystem: github-actions +# directory: "/" +# schedule: +# interval: daily +# time: "01:00" +# +# - package-ecosystem: npm +# directory: "/docs" +# schedule: +# interval: weekly +# # DevRel should review docs updates +# assignees: +# - "julienrbrt" +# +# - package-ecosystem: gomod +# directory: "/" +# schedule: +# interval: daily +# time: "01:05" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "/simapp" +# schedule: +# interval: daily +# time: "01:10" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "/tests" +# schedule: +# interval: weekly +# day: monday +# time: "01:15" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "/api" +# schedule: +# interval: weekly +# day: tuesday +# time: "01:20" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "/orm" +# schedule: +# interval: weekly +# day: wednesday +# time: "01:25" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "/core" +# schedule: +# interval: weekly +# day: thursday +# time: "01:30" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "/depinject" +# schedule: +# interval: weekly +# day: friday +# time: "01:35" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "/errors" +# schedule: +# interval: weekly +# day: monday +# time: "01:40" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "/math" +# schedule: +# interval: weekly +# day: tuesday +# time: "01:45" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "/client/v2" +# schedule: +# interval: weekly +# day: wednesday +# time: "01:50" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "/x/tx" +# schedule: +# interval: weekly +# day: thursday +# time: "01:55" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "/tools/cosmovisor" +# schedule: +# interval: weekly +# day: friday +# time: "02:00" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "/tools/confix" +# schedule: +# interval: weekly +# day: tuesday +# time: "02:10" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "/tools/hubl" +# schedule: +# interval: weekly +# day: thursday +# time: "02:15" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "/collections" +# schedule: +# interval: weekly +# day: friday +# time: "02:20" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "/x/nft" +# schedule: +# interval: weekly +# day: monday +# time: "02:25" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "/x/circuit" +# schedule: +# interval: weekly +# day: tuesday +# time: "02:30" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "x/feegrant" +# schedule: +# interval: weekly +# day: wednesday +# time: "02:35" +# labels: +# - "A:automerge" +# - dependencies +# +# - package-ecosystem: gomod +# directory: "/x/evidence" +# schedule: +# interval: weekly +# day: thursday +# time: "02:40" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "/store" +# schedule: +# interval: weekly +# day: friday +# time: "02:45" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "x/upgrade" +# schedule: +# interval: weekly +# day: monday +# time: "02:50" +# labels: +# - "A:automerge" +# - dependencies +# - package-ecosystem: gomod +# directory: "log" +# schedule: +# interval: weekly +# day: tuesday +# time: "02:55" +# labels: +# - "A:automerge" +# - dependencies +# +# # Dependencies should be up to date on release branch +# - package-ecosystem: gomod +# directory: "/" +# target-branch: "release/v0.47.x" +# schedule: +# interval: daily +# time: "03:00" +# labels: +# - "A:automerge" +# - dependencies +# - "testing-required" +# allow: +# - dependency-name: "github.com/cosmos/cosmos-sdk/*" +# dependency-type: "all" +# - dependency-name: "github.com/cosmos/*" +# dependency-type: "all" +# - dependency-name: "cosmossdk.io/*" +# dependency-type: "all" +# +# # Dependencies should be up to date on release branch +# - package-ecosystem: gomod +# directory: "/" +# target-branch: "release/v0.50.x" +# schedule: +# interval: daily +# time: "03:00" +# labels: +# - "A:automerge" +# - dependencies +# - "testing-required" +# allow: +# - dependency-name: "github.com/cosmos/cosmos-sdk/*" +# dependency-type: "all" +# - dependency-name: "github.com/cosmos/*" +# dependency-type: "all" +# - dependency-name: "cosmossdk.io/*" +# dependency-type: "all" diff --git a/.github/workflows/clean-action-artifacts.yml b/.github/workflows/clean-action-artifacts.yml index b84b15d4bbd3..188c8e6793aa 100644 --- a/.github/workflows/clean-action-artifacts.yml +++ b/.github/workflows/clean-action-artifacts.yml @@ -1,17 +1,17 @@ -name: Remove GitHub Action Old Artifacts - -on: - schedule: - # Every day at 1am - - cron: "0 1 * * *" - -jobs: - remove-old-artifacts: - runs-on: ubuntu-latest - timeout-minutes: 30 - - steps: - - name: Remove old artifacts - uses: c-hive/gha-remove-artifacts@v1 - with: - age: "7 days" +#name: Remove GitHub Action Old Artifacts +# +#on: +# schedule: +# # Every day at 1am +# - cron: "0 1 * * *" +# +#jobs: +# remove-old-artifacts: +# runs-on: ubuntu-latest +# timeout-minutes: 30 +# +# steps: +# - name: Remove old artifacts +# uses: c-hive/gha-remove-artifacts@v1 +# with: +# age: "7 days" diff --git a/.github/workflows/sims-045.yml b/.github/workflows/sims-045.yml index c0761e3a46c7..7479daba3345 100644 --- a/.github/workflows/sims-045.yml +++ b/.github/workflows/sims-045.yml @@ -2,8 +2,8 @@ name: Sims release/0.45.x # Sims workflow runs multiple types of simulations (nondeterminism, import-export, after-import, multi-seed-short) # This workflow will run on all Pull Requests, if a .go, .mod or .sum file have been changed on: - schedule: - - cron: "0 0,12 * * *" +# schedule: +# - cron: "0 0,12 * * *" release: types: [published] diff --git a/.github/workflows/sims-046.yml b/.github/workflows/sims-046.yml index d344fe31dabb..9468d8fbe5a3 100644 --- a/.github/workflows/sims-046.yml +++ b/.github/workflows/sims-046.yml @@ -2,8 +2,8 @@ name: Sims release/0.46.x # Sims workflow runs multiple types of simulations (nondeterminism, import-export, after-import, multi-seed-short) # This workflow will run on all Pull Requests, if a .go, .mod or .sum file have been changed on: - schedule: - - cron: "0 0,12 * * *" +# schedule: +# - cron: "0 0,12 * * *" release: types: [published] diff --git a/.github/workflows/sims-047.yml b/.github/workflows/sims-047.yml index c1798be36857..229f370c7f8f 100644 --- a/.github/workflows/sims-047.yml +++ b/.github/workflows/sims-047.yml @@ -2,8 +2,8 @@ name: Sims release/0.47.x # Sims workflow runs multiple types of simulations (nondeterminism, import-export, after-import, multi-seed-short) # This workflow will run on all Pull Requests, if a .go, .mod or .sum file have been changed on: - schedule: - - cron: "0 0,12 * * *" +# schedule: +# - cron: "0 0,12 * * *" release: types: [published] diff --git a/.github/workflows/sims-nightly.yml b/.github/workflows/sims-nightly.yml index 5241516d2c2a..d6db7fe7e454 100644 --- a/.github/workflows/sims-nightly.yml +++ b/.github/workflows/sims-nightly.yml @@ -2,8 +2,8 @@ name: Sims Nightly (Long) # Release Sims workflow runs long-lived (multi-seed & large block size) simulations # This workflow only runs mightly at 8am UTC and on releases on: - schedule: - - cron: "0 8 * * *" +# schedule: +# - cron: "0 8 * * *" release: types: [published] diff --git a/.github/workflows/sims.yml b/.github/workflows/sims.yml index 972241bd0819..93f83663e919 100644 --- a/.github/workflows/sims.yml +++ b/.github/workflows/sims.yml @@ -2,8 +2,8 @@ name: Sims # Sims workflow runs multiple types of simulations (nondeterminism, import-export, after-import, multi-seed-short) # This workflow will run on all Pull Requests, if a .go, .mod or .sum file have been changed on: - schedule: - - cron: "0 */2 * * *" +# schedule: +# - cron: "0 */2 * * *" release: types: [published] diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml index 20630a137610..fe1c179d48f7 100644 --- a/.github/workflows/stale.yml +++ b/.github/workflows/stale.yml @@ -1,26 +1,26 @@ -name: "Close stale issues & pull requests" -on: - schedule: - - cron: "0 0 * * *" - -permissions: - contents: read - -jobs: - stale: - permissions: - issues: write # for actions/stale to close stale issues - pull-requests: write # for actions/stale to close stale PRs - runs-on: ubuntu-latest - steps: - - uses: actions/stale@v8 - with: - repo-token: ${{ secrets.GITHUB_TOKEN }} - stale-pr-message: "This pull request has been automatically marked as stale because it has not had - recent activity. It will be closed if no further activity occurs. Thank you - for your contributions." - days-before-stale: -1 - days-before-close: -1 - days-before-pr-stale: 30 - days-before-pr-close: 4 - exempt-pr-labels: "pinned, security, proposal, blocked" +#name: "Close stale issues & pull requests" +#on: +# schedule: +# - cron: "0 0 * * *" +# +#permissions: +# contents: read +# +#jobs: +# stale: +# permissions: +# issues: write # for actions/stale to close stale issues +# pull-requests: write # for actions/stale to close stale PRs +# runs-on: ubuntu-latest +# steps: +# - uses: actions/stale@v8 +# with: +# repo-token: ${{ secrets.GITHUB_TOKEN }} +# stale-pr-message: "This pull request has been automatically marked as stale because it has not had +# recent activity. It will be closed if no further activity occurs. Thank you +# for your contributions." +# days-before-stale: -1 +# days-before-close: -1 +# days-before-pr-stale: 30 +# days-before-pr-close: 4 +# exempt-pr-labels: "pinned, security, proposal, blocked" diff --git a/client/keys/show_test.go b/client/keys/show_test.go index 2ac402a949ac..28312aba38cd 100644 --- a/client/keys/show_test.go +++ b/client/keys/show_test.go @@ -37,7 +37,7 @@ func Test_multiSigKey_Properties(t *testing.T) { addr, err := k.GetAddress() require.NoError(t, err) - require.Equal(t, "cosmos16wfryel63g7axeamw68630wglalcnk3l0zuadc", sdk.MustHexifyAddressBytes("cosmos", addr)) + require.Equal(t, "cosmos16wfryel63g7axeamw68630wglalcnk3l0zuadc", sdk.MustHexifyAddressBytes(addr)) } func Test_showKeysCmd(t *testing.T) { @@ -186,3 +186,36 @@ func Test_validateMultisigThreshold(t *testing.T) { }) } } + +// TODO HV2: removed as irrelevant to Heimdall +/* +func Test_getBechKeyOut(t *testing.T) { + type args struct { + bechPrefix string + } + tests := []struct { + name string + args args + want bechKeyOutFn + wantErr bool + }{ + {"empty", args{""}, nil, true}, + {"wrong", args{"???"}, nil, true}, + {"acc", args{sdk.PrefixAccount}, MkAccKeyOutput, false}, + {"val", args{sdk.PrefixValidator}, MkValKeyOutput, false}, + {"cons", args{sdk.PrefixConsensus}, MkConsKeyOutput, false}, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + got, err := getBechKeyOut(tt.args.bechPrefix) + if tt.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + require.NotNil(t, got) + } + }) + } +} +*/ diff --git a/types/address.go b/types/address.go index 81484cc843d9..01093d8af698 100644 --- a/types/address.go +++ b/types/address.go @@ -431,7 +431,7 @@ func (ca ConsAddress) String() string { // HexifyAddressBytes returns a hex representation of address bytes. // Returns an empty string if the byte slice is 0-length. Returns an error if the hex conversion // fails or the prefix is empty. -func HexifyAddressBytes(_ string, bs []byte) (string, error) { +func HexifyAddressBytes(bs []byte) (string, error) { if len(bs) == 0 { return "", nil } @@ -441,8 +441,8 @@ func HexifyAddressBytes(_ string, bs []byte) (string, error) { // MustHexifyAddressBytes returns a hex representation of address bytes. // Returns an empty sting if the byte slice is 0-length. It panics if the hex conversion // fails or the prefix is empty. -func MustHexifyAddressBytes(prefix string, bs []byte) string { - s, err := HexifyAddressBytes(prefix, bs) +func MustHexifyAddressBytes(bs []byte) string { + s, err := HexifyAddressBytes(bs) if err != nil { panic(err) } @@ -467,7 +467,7 @@ func (ca ConsAddress) Format(s fmt.State, verb rune) { // ---------------------------------------------------------------------------- // GetFromHex decodes a bytestring from a hex encoded string. -func GetFromHex(hexStr, _ string) ([]byte, error) { +func GetFromHex(hexStr string) ([]byte, error) { return addressBytesFromHexString(hexStr) } diff --git a/types/address_test.go b/types/address_test.go index 9f5d583bf89c..f83bae0a4dc4 100644 --- a/types/address_test.go +++ b/types/address_test.go @@ -16,7 +16,6 @@ import ( "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/types" - //nolint:staticcheck ) type addressTestSuite struct { @@ -250,6 +249,65 @@ func RandString(n int) string { return string(b) } +// TODO HV2: removed as irrelevant to Heimdall +/* +func (s *addressTestSuite) TestConfiguredPrefix() { + pubBz := make([]byte, ed25519.PubKeySize) + pub := &ed25519.PubKey{Key: pubBz} + for length := 1; length < 10; length++ { + for times := 1; times < 20; times++ { + rand.Read(pub.Key[:]) + // Test if randomly generated prefix of a given length works + prefix := RandString(length) + + // Assuming that GetConfig is not sealed. + config := types.GetConfig() + config.SetBech32PrefixForAccount( + prefix+types.PrefixAccount, + prefix+types.PrefixPublic) + + acc := types.AccAddress(pub.Address()) + s.Require().True(strings.HasPrefix( + acc.String(), + prefix+types.PrefixAccount), acc.String()) + + bech32Pub := legacybech32.MustMarshalPubKey(legacybech32.AccPK, pub) //nolint:staticcheck // SA1019: legacybech32 is deprecated: use the bech32 package instead. + s.Require().True(strings.HasPrefix( + bech32Pub, + prefix+types.PrefixPublic)) + + config.SetBech32PrefixForValidator( + prefix+types.PrefixValidator+types.PrefixAddress, + prefix+types.PrefixValidator+types.PrefixPublic) + + val := types.ValAddress(pub.Address()) + s.Require().True(strings.HasPrefix( + val.String(), + prefix+types.PrefixValidator+types.PrefixAddress)) + + bech32ValPub := legacybech32.MustMarshalPubKey(legacybech32.ValPK, pub) //nolint:staticcheck // SA1019: legacybech32 is deprecated: use the bech32 package instead. + s.Require().True(strings.HasPrefix( + bech32ValPub, + prefix+types.PrefixValidator+types.PrefixPublic)) + + config.SetBech32PrefixForConsensusNode( + prefix+types.PrefixConsensus+types.PrefixAddress, + prefix+types.PrefixConsensus+types.PrefixPublic) + + cons := types.ConsAddress(pub.Address()) + s.Require().True(strings.HasPrefix( + cons.String(), + prefix+types.PrefixConsensus+types.PrefixAddress)) + + bech32ConsPub := legacybech32.MustMarshalPubKey(legacybech32.ConsPK, pub) //nolint:staticcheck // SA1019: legacybech32 is deprecated: use the bech32 package instead. + s.Require().True(strings.HasPrefix( + bech32ConsPub, + prefix+types.PrefixConsensus+types.PrefixPublic)) + } + } +} +*/ + func (s *addressTestSuite) TestAddressInterface() { pubBz := make([]byte, ed25519.PubKeySize) pub := &ed25519.PubKey{Key: pubBz} @@ -362,7 +420,7 @@ func (s *addressTestSuite) TestBech32ifyAddressBytes() { for _, tt := range tests { tt := tt s.T().Run(tt.name, func(t *testing.T) { - got, err := types.HexifyAddressBytes(tt.args.prefix, tt.args.bs) + got, err := types.HexifyAddressBytes(tt.args.bs) if (err != nil) != tt.wantErr { t.Errorf("Bech32ifyBytes() error = %v, wantErr %v", err, tt.wantErr) return @@ -396,10 +454,10 @@ func (s *addressTestSuite) TestMustBech32ifyAddressBytes() { tt := tt s.T().Run(tt.name, func(t *testing.T) { if tt.wantPanic { - require.Panics(t, func() { types.MustHexifyAddressBytes(tt.args.prefix, tt.args.bs) }) + require.Panics(t, func() { types.MustHexifyAddressBytes(tt.args.bs) }) return } - require.Equal(t, tt.want, types.MustHexifyAddressBytes(tt.args.prefix, tt.args.bs)) + require.Equal(t, tt.want, types.MustHexifyAddressBytes(tt.args.bs)) }) } } @@ -458,10 +516,10 @@ func (s *addressTestSuite) TestGetConsAddress() { } func (s *addressTestSuite) TestGetFromBech32() { - _, err := types.GetFromHex("", "prefix") + _, err := types.GetFromHex("") s.Require().Error(err) s.Require().Equal("decoding Bech32 address failed: must provide a non empty address", err.Error()) - _, err = types.GetFromHex("cosmos1qqqsyqcyq5rqwzqfys8f67", "x") + _, err = types.GetFromHex("cosmos1qqqsyqcyq5rqwzqfys8f67") s.Require().Error(err) s.Require().Equal("invalid Bech32 prefix; expected x, got cosmos", err.Error()) } diff --git a/types/bech32/legacybech32/pk.go b/types/bech32/legacybech32/pk.go index 4cc10757b4a6..1cc3c74c9080 100644 --- a/types/bech32/legacybech32/pk.go +++ b/types/bech32/legacybech32/pk.go @@ -47,9 +47,7 @@ func getPrefix(pkt Bech32PubKeyType) string { // Deprecated: UnmarshalPubKey returns a PublicKey from a bech32-encoded PublicKey with // a given key type. func UnmarshalPubKey(pkt Bech32PubKeyType, pubkeyStr string) (cryptotypes.PubKey, error) { - bech32Prefix := getPrefix(pkt) - - bz, err := sdk.GetFromHex(pubkeyStr, bech32Prefix) + bz, err := sdk.GetFromHex(pubkeyStr) if err != nil { return nil, err } diff --git a/types/errors/errors.go b/types/errors/errors.go index 5318d463924e..189946326b91 100644 --- a/types/errors/errors.go +++ b/types/errors/errors.go @@ -145,4 +145,10 @@ var ( // ErrPanic should only be set when we recovering from a panic ErrPanic = errorsmod.ErrPanic + + // ErrSetBlockProposer defines an error when setting the block proposer fails. + ErrSetBlockProposer = errorsmod.Register(RootCodespace, 43, "error setting block proposer") + + // ErrRemoveBlockProposer defines an error when removing the block proposer fails. + ErrRemoveBlockProposer = errorsmod.Register(RootCodespace, 44, "error removing block proposer") ) diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index f69991d97444..a7cfcaa4d218 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -1,6 +1,7 @@ package ante import ( + "bytes" "fmt" errorsmod "cosmossdk.io/errors" @@ -83,26 +84,27 @@ func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee } feePayer := feeTx.FeePayer() - // feeGranter := feeTx.FeeGranter() deductFeesFrom := feePayer - // TODO HV2: removed `FeeGranter` logic as we are not using it in heimdall. Is this ok? Do we want to use it instead of `FeeCollector`? + // TODO HV2: feeGranter set to nil as not used in heimdall + feeGranter := []byte(nil) // feeGranter := feeTx.FeeGranter() + // if feegranter set deduct fee from feegranter account. // this works with only when feegrant enabled. - //if feeGranter != nil { - // feeGranterAddr := sdk.AccAddress(feeGranter) - // - // if dfd.feegrantKeeper == nil { - // return sdkerrors.ErrInvalidRequest.Wrap("fee grants are not enabled") - // } else if !bytes.Equal(feeGranterAddr, feePayer) { - // err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranterAddr, feePayer, fee, sdkTx.GetMsgs()) - // if err != nil { - // return errorsmod.Wrapf(err, "%s does not allow to pay fees for %s", feeGranter, feePayer) - // } - // } - // - // deductFeesFrom = feeGranterAddr - //} + if feeGranter != nil { + feeGranterAddr := sdk.AccAddress(feeGranter) + + if dfd.feegrantKeeper == nil { + return sdkerrors.ErrInvalidRequest.Wrap("fee grants are not enabled") + } else if !bytes.Equal(feeGranterAddr, feePayer) { + err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranterAddr, feePayer, fee, sdkTx.GetMsgs()) + if err != nil { + return errorsmod.Wrapf(err, "%s does not allow to pay fees for %s", feeGranter, feePayer) + } + } + + deductFeesFrom = feeGranterAddr + } deductFeesFromAcc := dfd.accountKeeper.GetAccount(ctx, deductFeesFrom) if deductFeesFromAcc == nil { diff --git a/x/auth/ante/feegrant_test.go b/x/auth/ante/feegrant_test.go index 2b5a335533cf..5dc84468f975 100644 --- a/x/auth/ante/feegrant_test.go +++ b/x/auth/ante/feegrant_test.go @@ -2,6 +2,7 @@ package ante_test import ( "context" + "errors" "math/rand" "testing" "time" @@ -31,6 +32,7 @@ func TestDeductFeesNoDelegation(t *testing.T) { cases := map[string]struct { fee int64 valid bool + skip bool err error errMsg string malleate func(*AnteTestSuite) (signer TestAccount, feeAcc sdk.AccAddress) @@ -38,6 +40,7 @@ func TestDeductFeesNoDelegation(t *testing.T) { "paying with low funds": { fee: 50, valid: false, + skip: false, err: sdkerrors.ErrInsufficientFunds, malleate: func(suite *AnteTestSuite) (TestAccount, sdk.AccAddress) { accs := suite.CreateTestAccounts(1) @@ -49,6 +52,7 @@ func TestDeductFeesNoDelegation(t *testing.T) { "paying with good funds": { fee: 50, valid: true, + skip: false, malleate: func(suite *AnteTestSuite) (TestAccount, sdk.AccAddress) { accs := suite.CreateTestAccounts(1) suite.bankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), accs[0].acc.GetAddress(), authtypes.FeeCollectorName, gomock.Any()).Return(nil).Times(2) @@ -58,6 +62,7 @@ func TestDeductFeesNoDelegation(t *testing.T) { "paying with no account": { fee: 1, valid: false, + skip: false, err: sdkerrors.ErrUnknownAddress, malleate: func(suite *AnteTestSuite) (TestAccount, sdk.AccAddress) { // Do not register the account @@ -71,6 +76,7 @@ func TestDeductFeesNoDelegation(t *testing.T) { "no fee with real account": { fee: 0, valid: true, + skip: false, malleate: func(suite *AnteTestSuite) (TestAccount, sdk.AccAddress) { accs := suite.CreateTestAccounts(1) suite.bankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), accs[0].acc.GetAddress(), authtypes.FeeCollectorName, gomock.Any()).Return(nil).Times(2) @@ -80,6 +86,7 @@ func TestDeductFeesNoDelegation(t *testing.T) { "no fee with no account": { fee: 0, valid: false, + skip: false, err: sdkerrors.ErrUnknownAddress, malleate: func(suite *AnteTestSuite) (TestAccount, sdk.AccAddress) { // Do not register the account @@ -90,11 +97,69 @@ func TestDeductFeesNoDelegation(t *testing.T) { }, nil }, }, + "valid fee grant": { + // note: the original test said "valid fee grant with no account". + // this is impossible given that feegrant.GrantAllowance calls + // SetAccount for the grantee. + fee: 50, + valid: true, + skip: true, + malleate: func(suite *AnteTestSuite) (TestAccount, sdk.AccAddress) { + accs := suite.CreateTestAccounts(2) + + suite.feeGrantKeeper.EXPECT().UseGrantedFees(gomock.Any(), accs[1].acc.GetAddress(), accs[0].acc.GetAddress(), gomock.Any(), gomock.Any()).Return(nil).Times(2) + suite.bankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), accs[1].acc.GetAddress(), authtypes.FeeCollectorName, gomock.Any()).Return(nil).Times(2) + return accs[0], accs[1].acc.GetAddress() + }, + }, + "no fee grant": { + fee: 2, + valid: false, + skip: true, + err: sdkerrors.ErrNotFound, + malleate: func(suite *AnteTestSuite) (TestAccount, sdk.AccAddress) { + accs := suite.CreateTestAccounts(2) + suite.feeGrantKeeper.EXPECT(). + UseGrantedFees(gomock.Any(), accs[1].acc.GetAddress(), accs[0].acc.GetAddress(), gomock.Any(), gomock.Any()). + Return(sdkerrors.ErrNotFound.Wrap("fee-grant not found")). + Times(2) + return accs[0], accs[1].acc.GetAddress() + }, + }, + "allowance smaller than requested fee": { + fee: 50, + valid: false, + skip: true, + errMsg: "fee limit exceeded", + malleate: func(suite *AnteTestSuite) (TestAccount, sdk.AccAddress) { + accs := suite.CreateTestAccounts(2) + suite.feeGrantKeeper.EXPECT(). + UseGrantedFees(gomock.Any(), accs[1].acc.GetAddress(), accs[0].acc.GetAddress(), gomock.Any(), gomock.Any()). + Return(errors.New("fee limit exceeded")). + Times(2) + return accs[0], accs[1].acc.GetAddress() + }, + }, + "granter cannot cover allowed fee grant": { + fee: 50, + valid: false, + skip: true, + err: sdkerrors.ErrInsufficientFunds, + malleate: func(suite *AnteTestSuite) (TestAccount, sdk.AccAddress) { + accs := suite.CreateTestAccounts(2) + suite.feeGrantKeeper.EXPECT().UseGrantedFees(gomock.Any(), accs[1].acc.GetAddress(), accs[0].acc.GetAddress(), gomock.Any(), gomock.Any()).Return(nil).Times(2) + suite.bankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), accs[1].acc.GetAddress(), authtypes.FeeCollectorName, gomock.Any()).Return(sdkerrors.ErrInsufficientFunds).Times(2) + return accs[0], accs[1].acc.GetAddress() + }, + }, } for name, stc := range cases { tc := stc // to make scopelint happy t.Run(name, func(t *testing.T) { + if tc.skip { + t.Skip("skipping test as not relevant to Heimdall") + } suite := SetupTestSuite(t, false) protoTxCfg := tx.NewTxConfig(codec.NewProtoCodec(suite.encCfg.InterfaceRegistry), tx.DefaultSignModes) // this just tests our handler diff --git a/x/auth/ante/setup.go b/x/auth/ante/setup.go index 35ba2dc36308..b43e65704f6a 100644 --- a/x/auth/ante/setup.go +++ b/x/auth/ante/setup.go @@ -37,6 +37,12 @@ func (sud SetUpContextDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate return newCtx, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be GasTx") } + // TODO HV2: implement this check for milestones execution? + // Check whether the chain has reached the hard fork length to execute milestone msgs + // if ctx.BlockHeight() < helper.GetAalborgHardForkHeight() && (stdTx.Msg.Type() == checkpointTypes.EventTypeMilestone || stdTx.Msg.Type() == checkpointTypes.EventTypeMilestoneTimeout) { + // newCtx = SetGasMeter(simulate, ctx, 0) + // return newCtx, sdk.ErrTxDecode("error decoding transaction").Result(), true + // } newCtx = SetGasMeter(simulate, ctx, gasTx.GetGas()) if cp := ctx.ConsensusParams(); cp.Block != nil { diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index dd0ebb9440a0..514d4440e6bc 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -6,7 +6,6 @@ import ( "encoding/base64" "encoding/hex" "fmt" - "github.com/cosmos/cosmos-sdk/types/address" "math/big" "google.golang.org/protobuf/types/known/anypb" @@ -57,14 +56,6 @@ func init() { // This is where apps can define their own PubKey type SignatureVerificationGasConsumer = func(meter storetypes.GasMeter, sig signing.SignatureV2, params types.Params) error -// TODO HV2: MainTxMsg has no usage so far. Double check it by looking at heimdall's code. - -// MainTxMsg tx hash -type MainTxMsg interface { - GetTxHash() address.HeimdallHash - GetLogIndex() uint64 -} - // SetPubKeyDecorator sets PubKeys in context for any signer which does not already have pubkey set // PubKeys must be set in context for all signers before any other sigverify decorators run // CONTRACT: Tx must implement SigVerifiableTx interface diff --git a/x/auth/ante/validator_tx_fee.go b/x/auth/ante/validator_tx_fee.go index 6cbb2abb46b3..adba6093538c 100644 --- a/x/auth/ante/validator_tx_fee.go +++ b/x/auth/ante/validator_tx_fee.go @@ -23,7 +23,7 @@ func checkTxFeeWithValidatorMinGasPrices(ctx sdk.Context, tx sdk.Tx, params type gas := params.GetMaxTxGas() feeCoins := sdk.Coins{sdk.Coin{Denom: types.FeeToken, Amount: amount}} - // TODO HV2: removed as not present in heimdall. Is this safe? Would it be better/safer to implement it? + // TODO HV2: removed as not present in heimdall // Ensure that the provided fees meet a minimum threshold for the validator, // if this is a CheckTx. This is only for local mempool purposes, and thus // is only ran on check tx. diff --git a/x/auth/keeper/keeper.go b/x/auth/keeper/keeper.go index 4a45dc9df09f..01e906e3df9d 100644 --- a/x/auth/keeper/keeper.go +++ b/x/auth/keeper/keeper.go @@ -275,20 +275,31 @@ func (ak AccountKeeper) GetBlockProposer(ctx sdk.Context) (sdk.AccAddress, bool) kvStore := ak.storeService.OpenKVStore(ctx) isProposerPresent, _ := kvStore.Has(types.ProposerKey()) if !isProposerPresent { - return nil, false + return sdk.AccAddress{}, false + } + blockProposerBytes, err := kvStore.Get(types.ProposerKey()) + if err != nil { + return sdk.AccAddress{}, false } - blockProposerBytes, _ := kvStore.Get(types.ProposerKey()) return blockProposerBytes, true } // SetBlockProposer sets block proposer -func (ak AccountKeeper) SetBlockProposer(ctx sdk.Context, addr sdk.AccAddress) { +func (ak AccountKeeper) SetBlockProposer(ctx sdk.Context, addr sdk.AccAddress) error { kvStore := ak.storeService.OpenKVStore(ctx) - kvStore.Set(types.ProposerKey(), addr.Bytes()) + err := kvStore.Set(types.ProposerKey(), addr.Bytes()) + if err != nil { + return errorsmod.Wrapf(sdkerrors.ErrSetBlockProposer, "account %s could not be set as block proposer", addr.String()) + } + return nil } // RemoveBlockProposer removes block proposer from store -func (ak AccountKeeper) RemoveBlockProposer(ctx sdk.Context) { +func (ak AccountKeeper) RemoveBlockProposer(ctx sdk.Context) error { kvStore := ak.storeService.OpenKVStore(ctx) - kvStore.Delete(types.ProposerKey()) + err := kvStore.Delete(types.ProposerKey()) + if err != nil { + return errorsmod.Wrapf(sdkerrors.ErrRemoveBlockProposer, "block proposer could not be removed") + } + return nil } diff --git a/x/auth/keeper/keeper_test.go b/x/auth/keeper/keeper_test.go index 21fbc3d8bb5a..b2c459c9601b 100644 --- a/x/auth/keeper/keeper_test.go +++ b/x/auth/keeper/keeper_test.go @@ -225,3 +225,34 @@ func (suite *KeeperTestSuite) TestInitGenesis() { // we expect nextNum to be 2 because we initialize fee_collector as account number 1 suite.Require().Equal(2, int(nextNum)) } + +func (suite *KeeperTestSuite) TestProposer() { + suite.SetupTest() + + proposer, ok := suite.accountKeeper.GetBlockProposer(suite.ctx) + suite.Require().False(ok) + suite.Require().Equal(sdk.AccAddress{}.Bytes(), proposer.Bytes()) + + testAddress, err := sdk.GetFromHex("0x9f86D081884C7d659A2fEaA0C55AD015A3bf4F1B") + suite.Require().NoError(err) + + // set addr as proposer + address, err := sdk.HexifyAddressBytes(testAddress) + suite.Require().NoError(err) + proposer, err = sdk.AccAddressFromHex(address) + suite.Require().NoError(err) + err = suite.accountKeeper.SetBlockProposer(suite.ctx, proposer) + suite.Require().NoError(err) + + proposer, ok = suite.accountKeeper.GetBlockProposer(suite.ctx) + suite.Require().True(ok) + suite.Require().Equal(testAddress, proposer.Bytes()) + + // remove block proposer + err = suite.accountKeeper.RemoveBlockProposer(suite.ctx) + suite.Require().NoError(err) + + proposer, ok = suite.accountKeeper.GetBlockProposer(suite.ctx) + suite.Require().False(ok) + suite.Require().Equal(sdk.AccAddress{}.Bytes(), proposer.Bytes()) +} diff --git a/x/auth/types/account.go b/x/auth/types/account.go index b39d12690cba..aa7d7d61769a 100644 --- a/x/auth/types/account.go +++ b/x/auth/types/account.go @@ -54,7 +54,10 @@ func NewBaseAccountWithAddress(addr sdk.AccAddress) *BaseAccount { // GetAddress - Implements sdk.AccountI. func (acc BaseAccount) GetAddress() sdk.AccAddress { - addr, _ := sdk.AccAddressFromHex(acc.Address) + addr, err := sdk.AccAddressFromHex(acc.Address) + if err != nil { + return sdk.AccAddress{} + } return addr } diff --git a/x/auth/types/params.go b/x/auth/types/params.go index 1862c43a1c38..052da599ca55 100644 --- a/x/auth/types/params.go +++ b/x/auth/types/params.go @@ -13,12 +13,12 @@ const ( DefaultTxSizeCostPerByte uint64 = 10 DefaultSigVerifyCostED25519 uint64 = 590 DefaultSigVerifyCostSecp256k1 uint64 = 1000 - DefaultMaxTxGas uint64 = 1000000 - DefaultTxFees string = "1000000000000000" + DefaultMaxTxGas uint64 = 1000000 + DefaultTxFees string = "1000000000000000" ) // NewParams creates a new Params object -func NewParams(maxMemoCharacters, txSigLimit, txSizeCostPerByte, sigVerifyCostED25519, sigVerifyCostSecp256k1,maxTxGas uint64, txFees string) Params { +func NewParams(maxMemoCharacters, txSigLimit, txSizeCostPerByte, sigVerifyCostED25519, sigVerifyCostSecp256k1, maxTxGas uint64, txFees string) Params { return Params{ MaxMemoCharacters: maxMemoCharacters, TxSigLimit: txSigLimit, @@ -123,7 +123,6 @@ func validateTxSizeCostPerByte(i interface{}) error { func validateMaxTxGas(i interface{}) error { v, ok := i.(uint64) if !ok { - return fmt.Errorf("invalid parameter type: %T", i) } diff --git a/x/group/keeper/msg_server.go b/x/group/keeper/msg_server.go index 5f33a42e0689..b36215a75e60 100644 --- a/x/group/keeper/msg_server.go +++ b/x/group/keeper/msg_server.go @@ -3,14 +3,13 @@ package keeper import ( "bytes" "context" + "encoding/binary" "encoding/json" "fmt" "strings" errorsmod "cosmossdk.io/errors" - "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" - sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" @@ -374,14 +373,9 @@ func (k Keeper) CreateGroupPolicy(goCtx context.Context, msg *group.MsgCreateGro // loop here in the rare case where a ADR-028-derived address creates a // collision with an existing address. for { - // TODO HV2: this code should work, but I believe it breaks the intended functionality of the group module. - // This implementation is here only for compatibility and allow the tests to pass. - // Otherwise, NewBaseAccountWithPubKey(pubKey) would fail with any derivationKey. - // This is because it calls NewBaseAccountWithAddress, which validates the address to be ethereum hex compatible - // We are not going to use group module (as we don't use multisig or accounts policies). - // However, is there a better alternative which keeps the group module operational? - pubKey := secp256k1.GenPrivKey().PubKey() - derivationKey := pubKey.Address() + nextAccVal := k.groupPolicySeq.NextVal(ctx.KVStore(k.key)) + derivationKey := make([]byte, 8) + binary.BigEndian.PutUint64(derivationKey, nextAccVal) ac, err := authtypes.NewModuleCredential(group.ModuleName, []byte{GroupPolicyTablePrefix}, derivationKey) if err != nil { @@ -395,7 +389,7 @@ func (k Keeper) CreateGroupPolicy(goCtx context.Context, msg *group.MsgCreateGro } // group policy accounts are unclaimable base accounts - account, err := authtypes.NewBaseAccountWithPubKey(pubKey) + account, err := authtypes.NewBaseAccountWithPubKey(ac) if err != nil { return nil, errorsmod.Wrap(err, "could not create group policy account") } diff --git a/x/group/migrations/v2/gen_state.go b/x/group/migrations/v2/gen_state.go index 1f5846c6140a..a21b018f1ecb 100644 --- a/x/group/migrations/v2/gen_state.go +++ b/x/group/migrations/v2/gen_state.go @@ -1,7 +1,8 @@ package v2 import ( - "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" + "encoding/binary" + sdk "github.com/cosmos/cosmos-sdk/types" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" ) @@ -28,20 +29,16 @@ func MigrateGenState(oldState *authtypes.GenesisState) *authtypes.GenesisState { continue } - // TODO HV2: this code should work, but I believe it breaks the intended functionality of the group module. - // This implementation is here only for compatibility and allow the tests to pass. - // Otherwise, NewBaseAccountWithPubKey(pubKey) would fail with any derivationKey. - // This is because it calls NewBaseAccountWithAddress, which validates the address to be ethereum hex compatible - // We are not going to use group module (as we don't use multisig or accounts policies). - // However, is there a better alternative which keeps the group module operational? - pubKey := secp256k1.GenPrivKey().PubKey() - derivationKey := pubKey.Address() + // Replace group policy accounts from module accounts to base accounts. + // These accounts were wrongly created and the address was equal to the module name. + derivationKey := make([]byte, 8) + binary.BigEndian.PutUint64(derivationKey, groupPolicyAccountCounter) - _, err = authtypes.NewModuleCredential(ModuleName, []byte{GroupPolicyTablePrefix}, derivationKey) + cred, err := authtypes.NewModuleCredential(ModuleName, []byte{GroupPolicyTablePrefix}, derivationKey) if err != nil { panic(err) } - baseAccount, err := authtypes.NewBaseAccountWithPubKey(pubKey) + baseAccount, err := authtypes.NewBaseAccountWithPubKey(cred) if err != nil { panic(err) } diff --git a/x/group/migrations/v2/migrate.go b/x/group/migrations/v2/migrate.go index 9cfb44a8804c..550ae51802a3 100644 --- a/x/group/migrations/v2/migrate.go +++ b/x/group/migrations/v2/migrate.go @@ -1,12 +1,9 @@ package v2 import ( + "encoding/binary" "fmt" - "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" - - cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" - storetypes "cosmossdk.io/store/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -35,21 +32,13 @@ func Migrate( ) error { store := ctx.KVStore(storeKey) curAccVal := groupPolicySeq.CurVal(store) - groupPolicyAccountPubKey := make(map[string]cryptotypes.PubKey, 0) groupPolicyAccountDerivationKey := make(map[string][]byte, 0) policyKey := []byte{GroupPolicyTablePrefix} for i := uint64(0); i <= curAccVal; i++ { - // TODO HV2: this code should work, but I believe it breaks the intended functionality of the group module. - // This implementation is here only for compatibility and allow the tests to pass. - // Otherwise, NewBaseAccountWithPubKey(pubKey) would fail with any derivationKey. - // This is because it calls NewBaseAccountWithAddress, which validates the address to be ethereum hex compatible - // We are not going to use group module (as we don't use multisig or accounts policies). - // However, is there a better alternative which keeps the group module operational? - pubKey := secp256k1.GenPrivKey().PubKey() - derivationKey := pubKey.Address() + derivationKey := make([]byte, 8) + binary.BigEndian.PutUint64(derivationKey, i) groupPolicyAcc := sdk.AccAddress(address.Module(group.ModuleName, policyKey, derivationKey)) groupPolicyAccountDerivationKey[groupPolicyAcc.String()] = derivationKey - groupPolicyAccountPubKey[groupPolicyAcc.String()] = pubKey } // get all group policies @@ -76,11 +65,11 @@ func Migrate( panic(fmt.Errorf("group policy account %s derivation key not found", policy.Address)) } - _, err = authtypes.NewModuleCredential(group.ModuleName, []byte{GroupPolicyTablePrefix}, derivationKey) + ac, err := authtypes.NewModuleCredential(group.ModuleName, []byte{GroupPolicyTablePrefix}, derivationKey) if err != nil { return err } - baseAccount, err := authtypes.NewBaseAccountWithPubKey(groupPolicyAccountPubKey[policy.Address]) + baseAccount, err := authtypes.NewBaseAccountWithPubKey(ac) if err != nil { return fmt.Errorf("failed to create new group policy account: %w", err) }