Skip to content

Commit

Permalink
feat: update x/bankplus for deprecation (#1223)
Browse files Browse the repository at this point in the history
* ci: fix pr-labeler

* feat: add bankplus

* chore: temporal removal of ci

* chore: fix for gosec

* chore: add bankplus module to go.work.example

* chore: update module for DI

* chore: go mod tidy

* chore: temporal fix for lint

* chore: temporal fix for lint

* chore: should fail

* chore: test-cosmossdk.io/errors

* chore: test-cosmossdk.io/store

* chore: test-cosmossdk.io/core

* chore: should fix lint fail after cosmossdk.io/corev0.12

* chore: after go tidy

* chore: add tests

* Revert "chore: temporal removal of ci"

This reverts commit 630804b.

* Revert "ci: fix pr-labeler"

This reverts commit 587d2da.

* chore: replace cometbft

* chore: add bankplus to pr_labeler

* chore: update CHANGELOG

* chore: add bankplus test ci

* chore: fix

* chore: add bankplus module to replace bank module

* chore: go mod tidy for tests

* chore: remove code that use global codec

* Update .github/workflows/test.yml

Co-authored-by: Shogo Hyodo <mmoshg8u@gmail.com>

* chore: remove code that use global codec from tests

* chore: introduce DeactMultiSend type and DI provider

* chore: fix tests

* chore: fix init order to apply link prefix codec

* chore: fix side-effect from original bank.send

* chore: remove empty bullet

* chore: replace filter logic with SendRestrictionFn filter

* chore: no need to check empty balance. Balance will be back while handling tx which throws error

* chore: remove duplicate private method

* chore: delete files for deprecation

* chore: add deprecation function for bankplus

* chore: remove unused dependency

* chore: update changelog

* chore: fix lint

* chore: add all migrator from original bank modules

* chore: reorder

* chore: clean-up

* chore: deprecate x/bankplus by moving deprecation logic to simapp

* chore: rollback original bank module tests in TestRunMigrations

* chore: keep bankplus in x/bankplus for history

* chore: fix for buf breaking

* chore: fix lint

* chore: organize go.mod

* chore: add test job for bankplus

* chore: refactor to accept kvStoreService instead of key

* chore: refactor delete logic not to touch iteration-related things while iteration

---------

Co-authored-by: Shogo Hyodo <mmoshg8u@gmail.com>
  • Loading branch information
jaeseung-bae and ulbqb committed Mar 27, 2024
1 parent 9db31c4 commit 7c39d83
Show file tree
Hide file tree
Showing 13 changed files with 1,734 additions and 21 deletions.
2 changes: 2 additions & 0 deletions .github/pr_labeler.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,5 @@
- x/collection/**/*
"C:x/foundation":
- x/foundation/**/*
"C:x/bankplus":
- x/bankplus/**/*
31 changes: 31 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -323,3 +323,34 @@ jobs:
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
with:
projectBaseDir: x/foundation/

test-x-bankplus:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/setup-go@v4
with:
go-version: "1.21"
check-latest: true
cache: true
cache-dependency-path: x/bankplus/go.sum
- uses: technote-space/get-diff-action@v6.1.2
id: git_diff
with:
PATTERNS: |
x/bankplus/**/*.go
x/bankplus/go.mod
x/bankplus/go.sum
- name: tests
if: env.GIT_DIFF
run: |
cd x/bankplus
go test -mod=readonly -timeout 30m -coverprofile=coverage.out -covermode=atomic -tags='norace ledger test_ledger_mock rocksdb_build' ./...
- name: sonarcloud
if: ${{ env.GIT_DIFF && !github.event.pull_request.draft && env.SONAR_TOKEN != null }}
uses: SonarSource/sonarcloud-github-action@master
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
with:
projectBaseDir: x/bankplus/
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/foundation) [\#1198](https://github.com/Finschia/finschia-sdk/pull/1198) update x/foundation to use Finschia/cosmos-sdk
* (all) [\#1205](https://github.com/Finschia/finschia-sdk/pull/1205) delegate native logics to Finschia/cosmos-sdk
* (consensus) [\#1178](https://github.com/Finschia/finschia-sdk/pull/1178) change the consensus from Ostracon to Tendermint v0.34.24
* (x/bankplus) [\#1223](https://github.com/Finschia/finschia-sdk/pull/1223) deprecate x/bankplus

### Improvements
* (docs) [\#1120](https://github.com/Finschia/finschia-sdk/pull/1120) Update links in x/foundation README.md
Expand All @@ -64,7 +65,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Removed

### Breaking Changes
* (consensus) [\#1178](https://github.com/Finschia/finschia-sdk/pull/1178) change the consensus from Ostracon to Tendermint v0.34.24
* (consensus) [\#1178](https://github.com/Finschia/finschia-sdk/pull/1178) change the consensus from Ostracon to Tendermint v0.34.24

### State Machine Breaking

Expand Down
28 changes: 15 additions & 13 deletions api/lbm/bankplus/v1/bankplus.pulsar.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@ module github.com/Finschia/finschia-sdk

require (
cosmossdk.io/math v1.2.0
github.com/cosmos/cosmos-proto v1.0.0-beta.3
github.com/cosmos/cosmos-sdk v0.50.2
github.com/cosmos/go-bip39 v1.0.0
github.com/cosmos/gogoproto v1.4.11
github.com/stretchr/testify v1.8.4
github.com/tendermint/go-amino v0.16.0
)
Expand Down Expand Up @@ -38,6 +36,8 @@ require (
github.com/cometbft/cometbft-db v0.9.1 // indirect
github.com/cosmos/btcutil v1.0.5 // indirect
github.com/cosmos/cosmos-db v1.0.0 // indirect
github.com/cosmos/cosmos-proto v1.0.0-beta.3 // indirect
github.com/cosmos/gogoproto v1.4.11 // indirect
github.com/cosmos/ics23/go v0.10.0 // indirect
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0 // indirect
Expand Down
1 change: 1 addition & 0 deletions go.work.example
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ use (
./tests
./x/collection
./x/foundation
./x/bankplus
)
1 change: 1 addition & 0 deletions proto/lbm/bankplus/v1/bankplus.proto
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ option go_package = "github.com/Finschia/finschia-sdk/x/bankplus/types";

// InactiveAddr models the blocked address for the bankplus module
message InactiveAddr {
option deprecated = true;
option (gogoproto.equal) = true;

string address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
Expand Down
9 changes: 4 additions & 5 deletions tests/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ require (

require (
github.com/Finschia/finschia-sdk/simapp v0.0.0-00010101000000-000000000000
github.com/Finschia/finschia-sdk/x/collection v0.0.0-00010101000000-000000000000
github.com/Finschia/finschia-sdk/x/foundation v0.0.0-00010101000000-000000000000
github.com/cosmos/go-bip39 v1.0.0
)
Expand All @@ -50,7 +51,6 @@ require (
github.com/DataDog/datadog-go v3.2.0+incompatible // indirect
github.com/DataDog/zstd v1.5.5 // indirect
github.com/Finschia/finschia-sdk/api v0.0.0-20231227090232-78fde403b78c // indirect
github.com/Finschia/finschia-sdk/x/collection v0.0.0-00010101000000-000000000000 // indirect
github.com/aws/aws-sdk-go v1.44.224 // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d // indirect
Expand Down Expand Up @@ -219,16 +219,15 @@ replace (
github.com/Finschia/finschia-sdk/api => ../api
// We always want to test against the latest version of the simapp.
github.com/Finschia/finschia-sdk/simapp => ../simapp

// Fix upstream GHSA-h395-qcrw-5vmq and GHSA-3vp4-m3rf-835h vulnerabilities.
// TODO Remove it: https://github.com/cosmos/cosmos-sdk/issues/10409
github.com/gin-gonic/gin => github.com/gin-gonic/gin v1.9.1
)

replace (
github.com/cometbft/cometbft => github.com/Finschia/cometbft v0.0.0-20231127181424-2aacfbe9832d
github.com/cosmos/cosmos-sdk => github.com/Finschia/cosmos-sdk v0.0.0-20231211060251-d8fb76d4c267
github.com/Finschia/finschia-sdk/x/collection => ./../x/collection

github.com/Finschia/finschia-sdk/x/foundation => ./../x/foundation
github.com/Finschia/finschia-sdk/x/collection => ./../x/collection
github.com/cometbft/cometbft => github.com/Finschia/cometbft v0.0.0-20231127181424-2aacfbe9832d
github.com/cosmos/cosmos-sdk => github.com/Finschia/cosmos-sdk v0.0.0-20231211060251-d8fb76d4c267
)
65 changes: 65 additions & 0 deletions x/bankplus/deprecator.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package bankplus

import (
"context"

"cosmossdk.io/core/store"
storetypes "cosmossdk.io/store/types"

"github.com/cosmos/cosmos-sdk/runtime"
sdk "github.com/cosmos/cosmos-sdk/types"
)

// inactiveAddrsKeyPrefix Keys for bankplus store but this prefix must not be overlap with bank key prefix.
var inactiveAddrsKeyPrefix = []byte{0xa0}

// inactiveAddrKey key of a specific inactiveAddr from store
func inactiveAddrKey(addr sdk.AccAddress) []byte {
return append(inactiveAddrsKeyPrefix, addr.Bytes()...)
}

// DeprecateBankPlus performs remove logic for bankplus v1.
// This will remove all the state(inactive addresses)
// This supposed to be called in simapp.
//
// Example) simapp/upgrades.go
//
// func (app SimApp) RegisterUpgradeHandlers() {
// app.UpgradeKeeper.SetUpgradeHandler(
// UpgradeName,
// func(ctx context.Context, _ upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) {
// app.deprecateBankPlusFromSimapp(ctx)
// return app.ModuleManager.RunMigrations(ctx, app.Configurator(), fromVM)
// },
// )
// ...
//
// func (app SimApp) deprecateBankPlusFromSimapp(ctx context.Context) {
// for _, key := range app.kvStoreKeys() {
// if key.Name() == banktypes.StoreKey {
// bankStoreService := runtime.NewKVStoreService(key)
// err := bankplus.DeprecateBankPlus(ctx, bankStoreService)
// if err != nil {
// panic(fmt.Errorf("failed to deprecate x/bankplus: %w", err))
// }
// }
// }
// }
func DeprecateBankPlus(ctx context.Context, bankStoreService store.KVStoreService) error {
kvStore := bankStoreService.OpenKVStore(ctx)
adapter := runtime.KVStoreAdapter(kvStore)
iter := storetypes.KVStorePrefixIterator(adapter, inactiveAddrsKeyPrefix)
defer iter.Close()

keys := [][]byte{}
for ; iter.Valid(); iter.Next() {
keys = append(keys, iter.Key())
}
for _, key := range keys {
err := kvStore.Delete(key)
if err != nil {
return err
}
}
return nil
}
93 changes: 93 additions & 0 deletions x/bankplus/deprecator_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package bankplus

import (
"context"
"testing"

cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
cmttime "github.com/cometbft/cometbft/types/time"
"github.com/stretchr/testify/suite"

"cosmossdk.io/core/address"
"cosmossdk.io/core/store"
storetypes "cosmossdk.io/store/types"

"github.com/cosmos/cosmos-sdk/codec"
codectestutil "github.com/cosmos/cosmos-sdk/codec/testutil"
"github.com/cosmos/cosmos-sdk/runtime"
"github.com/cosmos/cosmos-sdk/testutil"
sdk "github.com/cosmos/cosmos-sdk/types"
moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"

"github.com/Finschia/finschia-sdk/x/bankplus/types"
)

func TestDeprecateTestSuite(t *testing.T) {
suite.Run(t, &DeprecationTestSuite{})
}

type DeprecationTestSuite struct {
suite.Suite
ctx sdk.Context
cdc codec.Codec
storeService store.KVStoreService
key *storetypes.KVStoreKey
}

func (s *DeprecationTestSuite) SetupTest() {
s.key = storetypes.NewKVStoreKey(banktypes.StoreKey)
testCtx := testutil.DefaultContextWithDB(s.T(), s.key, storetypes.NewTransientStoreKey("transient_test"))
s.ctx = testCtx.Ctx.WithBlockHeader(cmtproto.Header{Time: cmttime.Now()})
encCfg := moduletestutil.MakeTestEncodingConfig()
encCfg.Codec = codectestutil.CodecOptions{
AccAddressPrefix: "link",
ValAddressPrefix: "linkvaloper",
}.NewCodec()
s.cdc = encCfg.Codec

storeService := runtime.NewKVStoreService(s.key)
s.storeService = storeService
}

func (s *DeprecationTestSuite) TestDeprecateBankPlus() {
oldAcc := authtypes.NewBaseAccountWithAddress(sdk.AccAddress("acc1"))
anotherOldAcc := authtypes.NewBaseAccountWithAddress(sdk.AccAddress("acc2"))
s.Require().False(isStoredInactiveAddr(s.ctx, s.storeService, oldAcc.GetAddress()))
s.Require().False(isStoredInactiveAddr(s.ctx, s.storeService, anotherOldAcc.GetAddress()))

addrCdc := s.cdc.InterfaceRegistry().SigningContext().AddressCodec()
addToInactiveAddr(s.ctx, s.storeService, s.cdc, addrCdc, oldAcc.GetAddress())
addToInactiveAddr(s.ctx, s.storeService, s.cdc, addrCdc, anotherOldAcc.GetAddress())
s.Require().True(isStoredInactiveAddr(s.ctx, s.storeService, oldAcc.GetAddress()))
s.Require().True(isStoredInactiveAddr(s.ctx, s.storeService, anotherOldAcc.GetAddress()))

err := DeprecateBankPlus(s.ctx, s.storeService)

s.Require().NoError(err)
s.Require().False(isStoredInactiveAddr(s.ctx, s.storeService, oldAcc.GetAddress()))
s.Require().False(isStoredInactiveAddr(s.ctx, s.storeService, anotherOldAcc.GetAddress()))
}

// isStoredInactiveAddr checks if the address is stored or not as blocked address
func isStoredInactiveAddr(ctx context.Context, storeService store.KVStoreService, address sdk.AccAddress) bool {
kvStore := storeService.OpenKVStore(ctx)
bz, _ := kvStore.Get(inactiveAddrKey(address))
return bz != nil
}

// addToInactiveAddr adds a blocked address to the store.
func addToInactiveAddr(ctx context.Context, storeService store.KVStoreService, cdc codec.Codec, addrCdc address.Codec, address sdk.AccAddress) {
kvStore := storeService.OpenKVStore(ctx)
addrString, err := addrCdc.BytesToString(address)
if err != nil {
panic(err)
}

blockedCAddr := types.InactiveAddr{Address: addrString} // nolint: staticcheck // SA1019 types.InactiveAddr is deprecated. Will be removed next version
bz := cdc.MustMarshal(&blockedCAddr)
if err := kvStore.Set(inactiveAddrKey(address), bz); err != nil {
panic(err)
}
}
Loading

0 comments on commit 7c39d83

Please sign in to comment.