From 4a05141902b2d48da30de8505009f4834e031803 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Wed, 9 Oct 2024 11:28:13 -0600 Subject: [PATCH] fix(vlocalchain): refuse to use existing account --- golang/cosmos/app/app.go | 1 + golang/cosmos/vm/action.go | 2 +- golang/cosmos/x/vlocalchain/keeper/keeper.go | 32 ++++++++++---- .../x/vlocalchain/keeper/keeper_test.go | 21 +++++---- .../x/vlocalchain/types/expected_keepers.go | 12 +++++ .../cosmos/x/vlocalchain/vlocalchain_test.go | 44 +++++++++++++++---- 6 files changed, 83 insertions(+), 29 deletions(-) create mode 100644 golang/cosmos/x/vlocalchain/types/expected_keepers.go diff --git a/golang/cosmos/app/app.go b/golang/cosmos/app/app.go index 52eb932eaa0..e5dc4a3c4fe 100644 --- a/golang/cosmos/app/app.go +++ b/golang/cosmos/app/app.go @@ -655,6 +655,7 @@ func NewAgoricApp( keys[vlocalchain.StoreKey], app.BaseApp.MsgServiceRouter(), app.BaseApp.GRPCQueryRouter(), + app.AccountKeeper, ) app.vlocalchainPort = app.AgdServer.MustRegisterPortHandler( "vlocalchain", diff --git a/golang/cosmos/vm/action.go b/golang/cosmos/vm/action.go index 349934e49d0..149be297f5d 100644 --- a/golang/cosmos/vm/action.go +++ b/golang/cosmos/vm/action.go @@ -86,7 +86,7 @@ func PopulateAction(ctx sdk.Context, action Action) Action { var headerPtr *ActionHeader if fieldType.Type == actionHeaderType { headerPtr = field.Addr().Interface().(*ActionHeader) - } else if fieldType.Type == reflect.PtrTo(actionHeaderType) { + } else if fieldType.Type == reflect.PointerTo(actionHeaderType) { if field.IsNil() { headerPtr = &ActionHeader{} } else { diff --git a/golang/cosmos/x/vlocalchain/keeper/keeper.go b/golang/cosmos/x/vlocalchain/keeper/keeper.go index b68893ba6b3..4afb6f859fe 100644 --- a/golang/cosmos/x/vlocalchain/keeper/keeper.go +++ b/golang/cosmos/x/vlocalchain/keeper/keeper.go @@ -28,6 +28,7 @@ type Keeper struct { cdc codec.Codec msgRouter types.MsgRouter queryRouter types.GRPCQueryRouter + acctKeeper types.AccountKeeper } // NewKeeper creates a new dIBC Keeper instance @@ -36,12 +37,14 @@ func NewKeeper( key storetypes.StoreKey, msgRouter types.MsgRouter, queryRouter types.GRPCQueryRouter, + acctKeeper types.AccountKeeper, ) Keeper { return Keeper{ key: key, cdc: cdc, msgRouter: msgRouter, queryRouter: queryRouter, + acctKeeper: acctKeeper, } } @@ -266,14 +269,27 @@ func (k Keeper) AllocateAddress(cctx context.Context) sdk.AccAddress { localchainModuleAcc := sdkaddress.Module(types.ModuleName, []byte("localchain")) header := ctx.BlockHeader() - // Increment our sequence number. - seq := store.Get(types.KeyLastSequence) - seq = types.NextSequence(seq) - store.Set(types.KeyLastSequence, seq) + // Loop until we find an unused address. + for { + // Increment our sequence number. + seq := store.Get(types.KeyLastSequence) + seq = types.NextSequence(seq) + store.Set(types.KeyLastSequence, seq) - buf := seq - buf = append(buf, header.AppHash...) - buf = append(buf, header.DataHash...) + buf := seq + buf = append(buf, header.AppHash...) + buf = append(buf, header.DataHash...) - return sdkaddress.Derive(localchainModuleAcc, buf) + addr := sdkaddress.Derive(localchainModuleAcc, buf) + if k.acctKeeper.HasAccount(ctx, addr) { + continue + } + + // We found an unused address, so create an account for it. + acct := k.acctKeeper.NewAccountWithAddress(ctx, addr) + k.acctKeeper.SetAccount(ctx, acct) + + // All good, return the address. + return addr + } } diff --git a/golang/cosmos/x/vlocalchain/keeper/keeper_test.go b/golang/cosmos/x/vlocalchain/keeper/keeper_test.go index e60887017e6..0e7333c08b9 100644 --- a/golang/cosmos/x/vlocalchain/keeper/keeper_test.go +++ b/golang/cosmos/x/vlocalchain/keeper/keeper_test.go @@ -10,7 +10,6 @@ import ( banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" ) - func TestKeeper_ParseRequestTypeURL(t *testing.T) { testCases := []struct { name string @@ -46,7 +45,7 @@ func TestKeeper_DeserializeTxMessages(t *testing.T) { banktypes.RegisterInterfaces(encodingConfig.InterfaceRegistry) - keeper := NewKeeper(cdc, nil, nil, nil) + keeper := NewKeeper(cdc, nil, nil, nil, nil) expectedMsgSend := []sdk.Msg{ &banktypes.MsgSend{ @@ -63,22 +62,22 @@ func TestKeeper_DeserializeTxMessages(t *testing.T) { wantErr bool }{ { - name: "camelCase keys", - json: `{"messages":[{"@type":"/cosmos.bank.v1beta1.MsgSend","fromAddress":"cosmos1abc","toAddress":"cosmos1xyz","amount":[{"denom":"stake","amount":"100"}]}]}`, + name: "camelCase keys", + json: `{"messages":[{"@type":"/cosmos.bank.v1beta1.MsgSend","fromAddress":"cosmos1abc","toAddress":"cosmos1xyz","amount":[{"denom":"stake","amount":"100"}]}]}`, expected: expectedMsgSend, - wantErr: false, + wantErr: false, }, { - name: "snake_case keys", - json: `{"messages":[{"@type":"/cosmos.bank.v1beta1.MsgSend","from_address":"cosmos1abc","to_address":"cosmos1xyz","amount":[{"denom":"stake","amount":"100"}]}]}`, + name: "snake_case keys", + json: `{"messages":[{"@type":"/cosmos.bank.v1beta1.MsgSend","from_address":"cosmos1abc","to_address":"cosmos1xyz","amount":[{"denom":"stake","amount":"100"}]}]}`, expected: expectedMsgSend, - wantErr: false, + wantErr: false, }, { - name: "misspelled key", - json: `{"messages":[{"@type":"/cosmos.bank.v1beta1.MsgSend","from_addresss":"cosmos1abc","to_address":"cosmos1xyz","amount":[{"denom":"stake","amount":"100"}]}]}`, + name: "misspelled key", + json: `{"messages":[{"@type":"/cosmos.bank.v1beta1.MsgSend","from_addresss":"cosmos1abc","to_address":"cosmos1xyz","amount":[{"denom":"stake","amount":"100"}]}]}`, expected: expectedMsgSend, - wantErr: true, + wantErr: true, }, } diff --git a/golang/cosmos/x/vlocalchain/types/expected_keepers.go b/golang/cosmos/x/vlocalchain/types/expected_keepers.go new file mode 100644 index 00000000000..a3fa5fdf43c --- /dev/null +++ b/golang/cosmos/x/vlocalchain/types/expected_keepers.go @@ -0,0 +1,12 @@ +package types + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +type AccountKeeper interface { + NewAccountWithAddress(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI + HasAccount(ctx sdk.Context, addr sdk.AccAddress) bool + SetAccount(ctx sdk.Context, acc authtypes.AccountI) +} diff --git a/golang/cosmos/x/vlocalchain/vlocalchain_test.go b/golang/cosmos/x/vlocalchain/vlocalchain_test.go index 57de979ff58..e428a2ae199 100644 --- a/golang/cosmos/x/vlocalchain/vlocalchain_test.go +++ b/golang/cosmos/x/vlocalchain/vlocalchain_test.go @@ -16,6 +16,7 @@ import ( "github.com/cosmos/cosmos-sdk/store" storetypes "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" transfertypes "github.com/cosmos/ibc-go/v6/modules/apps/transfer/types" @@ -36,6 +37,25 @@ const ( msgAllocateAddress = `{"type":"VLOCALCHAIN_ALLOCATE_ADDRESS"}` ) +type mockAccounts struct { + existing map[string]bool +} + +var _ types.AccountKeeper = (*mockAccounts)(nil) + +func (a *mockAccounts) NewAccountWithAddress(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI { + return authtypes.NewBaseAccountWithAddress(addr) +} + +func (a *mockAccounts) HasAccount(ctx sdk.Context, addr sdk.AccAddress) bool { + existing := a.existing[addr.String()] + return existing +} + +func (a *mockAccounts) SetAccount(ctx sdk.Context, acc authtypes.AccountI) { + a.existing[acc.GetAddress().String()] = true +} + type mockBank struct { banktypes.UnimplementedQueryServer banktypes.UnimplementedMsgServer @@ -108,7 +128,7 @@ func (s *mockStaking) UnbondingDelegation(cctx context.Context, req *stakingtype } // makeTestKit creates a minimal Keeper and Context for use in testing. -func makeTestKit(bank *mockBank, transfer *mockTransfer, staking *mockStaking) (vm.PortHandler, context.Context) { +func makeTestKit(bank *mockBank, transfer *mockTransfer, staking *mockStaking, accts *mockAccounts) (vm.PortHandler, context.Context) { encodingConfig := params.MakeEncodingConfig() cdc := encodingConfig.Marshaler @@ -127,9 +147,8 @@ func makeTestKit(bank *mockBank, transfer *mockTransfer, staking *mockStaking) ( stakingtypes.RegisterMsgServer(txRouter, staking) stakingtypes.RegisterQueryServer(queryRouter, staking) - // create a new Keeper - keeper := vlocalchain.NewKeeper(cdc, vlocalchainStoreKey, txRouter, queryRouter) + keeper := vlocalchain.NewKeeper(cdc, vlocalchainStoreKey, txRouter, queryRouter, accts) db := dbm.NewMemDB() ms := store.NewCommitMultiStore(db) @@ -153,13 +172,15 @@ func TestAllocateAddress(t *testing.T) { bank := &mockBank{} transfer := &mockTransfer{} staking := &mockStaking{} - handler, cctx := makeTestKit(bank, transfer, staking) + acct := &mockAccounts{existing: map[string]bool{ + firstAddr: true, + "cosmos1c5hplwyxk5jr2dsygjqepzfqvfukwduq9c4660aah76krf99m6gs0k7hvl": true, + }} + handler, cctx := makeTestKit(bank, transfer, staking, acct) addrs := map[string]bool{ - firstAddr: false, "cosmos1yj40fakym8kf4wvgz9tky7k9f3v9msm3t7frscrmkjsdkxkpsfkqgeczkg": false, "cosmos1s76vryj7m8k8nm9le65a4plhf5rym5sumtt2n0vwnk5l6k4cwuhsj56ujj": false, - "cosmos1c5hplwyxk5jr2dsygjqepzfqvfukwduq9c4660aah76krf99m6gs0k7hvl": false, "cosmos1ys3a7mtna3cad0wxcs4ddukn37stexjdvns8jfdn4uerlr95y4xqnrypf6": false, } numToTest := len(addrs) @@ -185,6 +206,9 @@ func TestAllocateAddress(t *testing.T) { t.Fatalf("unexpected duplicate address[%d]: %v", i, addr) } addrs[addr] = true + if !acct.existing[addr] { + t.Fatalf("expected address[%d]: %v to be added to accounts", i, addr) + } } } @@ -197,7 +221,8 @@ func TestQuery(t *testing.T) { }} transfer := &mockTransfer{} staking := &mockStaking{} - handler, cctx := makeTestKit(bank, transfer, staking) + accts := &mockAccounts{existing: map[string]bool{}} + handler, cctx := makeTestKit(bank, transfer, staking, accts) // get balances testCases := []struct { @@ -338,7 +363,8 @@ func TestExecuteTx(t *testing.T) { }} transfer := &mockTransfer{} staking := &mockStaking{} - handler, cctx := makeTestKit(bank, transfer, staking) + accts := &mockAccounts{existing: map[string]bool{}} + handler, cctx := makeTestKit(bank, transfer, staking, accts) // create a new message msg := `{"type":"VLOCALCHAIN_ALLOCATE_ADDRESS"}` @@ -426,7 +452,7 @@ func TestExecuteTx(t *testing.T) { if len(resp) != 1 { t.Fatalf("expected 1 response, got %d", len(resp)) } - + if _, ok := resp[0]["completionTime"]; !ok { t.Error("expected 'completionTime' field in response") }