Skip to content

Commit

Permalink
fix(vlocalchain): refuse to use existing account
Browse files Browse the repository at this point in the history
  • Loading branch information
michaelfig committed Oct 9, 2024
1 parent 3601b6c commit 4a05141
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 29 deletions.
1 change: 1 addition & 0 deletions golang/cosmos/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,7 @@ func NewAgoricApp(
keys[vlocalchain.StoreKey],
app.BaseApp.MsgServiceRouter(),
app.BaseApp.GRPCQueryRouter(),
app.AccountKeeper,
)
app.vlocalchainPort = app.AgdServer.MustRegisterPortHandler(
"vlocalchain",
Expand Down
2 changes: 1 addition & 1 deletion golang/cosmos/vm/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
32 changes: 24 additions & 8 deletions golang/cosmos/x/vlocalchain/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
}
}

Expand Down Expand Up @@ -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
}
}
21 changes: 10 additions & 11 deletions golang/cosmos/x/vlocalchain/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
)


func TestKeeper_ParseRequestTypeURL(t *testing.T) {
testCases := []struct {
name string
Expand Down Expand Up @@ -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{
Expand All @@ -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,
},
}

Expand Down
12 changes: 12 additions & 0 deletions golang/cosmos/x/vlocalchain/types/expected_keepers.go
Original file line number Diff line number Diff line change
@@ -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)
}
44 changes: 35 additions & 9 deletions golang/cosmos/x/vlocalchain/vlocalchain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
}
}
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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"}`
Expand Down Expand Up @@ -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")
}
Expand Down

0 comments on commit 4a05141

Please sign in to comment.