From 5b9c6592c2050a4bd9467b9a14f787f600567f30 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 21 Jan 2020 13:55:51 +0100 Subject: [PATCH 1/2] Test mask re-dispatching contract msg, fix bugs --- x/wasm/internal/keeper/keeper.go | 5 +- x/wasm/internal/keeper/mask_test.go | 94 ++++++++++++++++++++++++++--- 2 files changed, 87 insertions(+), 12 deletions(-) diff --git a/x/wasm/internal/keeper/keeper.go b/x/wasm/internal/keeper/keeper.go index e2e498b41..8ef371a29 100644 --- a/x/wasm/internal/keeper/keeper.go +++ b/x/wasm/internal/keeper/keeper.go @@ -308,11 +308,8 @@ func (k Keeper) dispatchMessage(ctx sdk.Context, contract exported.Account, msg if stderr != nil { return sdk.ErrInvalidAddress(msg.Contract.ContractAddr) } - // TODO: use non nil payment once we update go-cosmwasm (ContractMsg contains optional payment) _, err := k.Execute(ctx, targetAddr, contract.GetAddress(), []byte(msg.Contract.Msg), nil) - if err != nil { - return err - } + return err // may be nil } else if msg.Opaque != nil { msg, err := ParseOpaqueMsg(k.cdc, msg.Opaque) if err != nil { diff --git a/x/wasm/internal/keeper/mask_test.go b/x/wasm/internal/keeper/mask_test.go index e1be03c91..64aae4638 100644 --- a/x/wasm/internal/keeper/mask_test.go +++ b/x/wasm/internal/keeper/mask_test.go @@ -6,6 +6,7 @@ import ( "os" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" wasmTypes "github.com/confio/go-cosmwasm/types" @@ -29,7 +30,7 @@ type reflectPayload struct { Msg wasmTypes.CosmosMsg `json:"msg"` } -func TestMaskSend(t *testing.T) { +func TestMaskReflectOpaque(t *testing.T) { tempDir, err := ioutil.TempDir("", "wasm") require.NoError(t, err) defer os.RemoveAll(tempDir) @@ -61,7 +62,6 @@ func TestMaskSend(t *testing.T) { } transferBz, err := json.Marshal(transfer) require.NoError(t, err) - // TODO: switch order of args Instantiate vs Execute (caller/code vs contract/caller), (msg/coins vs coins/msg) _, err = keeper.Execute(ctx, contractAddr, creator, transferBz, nil) require.NoError(t, err) @@ -71,7 +71,6 @@ func TestMaskSend(t *testing.T) { checkAccount(t, ctx, accKeeper, fred, nil) // bob can send contract's tokens to fred (using SendMsg) - // TODO: fix this upstream msg := wasmTypes.CosmosMsg{ Send: &wasmTypes.SendMsg{ FromAddress: contractAddr.String(), @@ -89,7 +88,6 @@ func TestMaskSend(t *testing.T) { } reflectSendBz, err := json.Marshal(reflectSend) require.NoError(t, err) - // TODO: switch order of args Instantiate vs Execute (caller/code vs contract/caller), (msg/coins vs coins/msg) _, err = keeper.Execute(ctx, contractAddr, bob, reflectSendBz, nil) require.NoError(t, err) @@ -117,7 +115,6 @@ func TestMaskSend(t *testing.T) { reflectOpaqueBz, err := json.Marshal(reflectOpaque) require.NoError(t, err) - // TODO: switch order of args Instantiate vs Execute (caller/code vs contract/caller), (msg/coins vs coins/msg) _, err = keeper.Execute(ctx, contractAddr, bob, reflectOpaqueBz, nil) require.NoError(t, err) @@ -128,12 +125,93 @@ func TestMaskSend(t *testing.T) { checkAccount(t, ctx, accKeeper, bob, deposit) } +func TestMaskReflectContractSend(t *testing.T) { + tempDir, err := ioutil.TempDir("", "wasm") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + ctx, accKeeper, keeper := CreateTestInput(t, false, tempDir) + + deposit := sdk.NewCoins(sdk.NewInt64Coin("denom", 100000)) + creator := createFakeFundedAccount(ctx, accKeeper, deposit) + _, _, bob := keyPubAddr() + + // upload mask code + maskCode, err := ioutil.ReadFile("./testdata/mask.wasm") + require.NoError(t, err) + maskID, err := keeper.Create(ctx, creator, maskCode, "", "") + require.NoError(t, err) + require.Equal(t, uint64(1), maskID) + + // upload hackatom escrow code + escrowCode, err := ioutil.ReadFile("./testdata/contract.wasm") + require.NoError(t, err) + escrowID, err := keeper.Create(ctx, creator, escrowCode, "", "") + require.NoError(t, err) + require.Equal(t, uint64(2), escrowID) + + // creator instantiates a contract and gives it tokens + maskStart := sdk.NewCoins(sdk.NewInt64Coin("denom", 40000)) + maskAddr, err := keeper.Instantiate(ctx, maskID, creator, []byte("{}"), maskStart) + require.NoError(t, err) + require.NotEmpty(t, maskAddr) + + // now we set contract as verifier of an escrow + initMsg := InitMsg{ + Verifier: maskAddr, + Beneficiary: bob, + } + initMsgBz, err := json.Marshal(initMsg) + require.NoError(t, err) + escrowStart := sdk.NewCoins(sdk.NewInt64Coin("denom", 25000)) + escrowAddr, err := keeper.Instantiate(ctx, escrowID, creator, initMsgBz, escrowStart) + require.NoError(t, err) + require.NotEmpty(t, escrowAddr) + + // let's make sure all balances make sense + checkAccount(t, ctx, accKeeper, creator, sdk.NewCoins(sdk.NewInt64Coin("denom", 35000))) // 100k - 40k - 25k + checkAccount(t, ctx, accKeeper, maskAddr, maskStart) + checkAccount(t, ctx, accKeeper, escrowAddr, escrowStart) + checkAccount(t, ctx, accKeeper, bob, nil) + + // now for the trick.... we reflect a message through the mask to call the escrow + // we also send an additional 14k tokens there. + // this should reduce the mask balance by 14k (to 26k) + // this 14k is added to the escrow, then the entire balance is sent to bob (total: 39k) + approveMsg := "{}" + msg := wasmTypes.CosmosMsg{ + Contract: &wasmTypes.ContractMsg{ + ContractAddr: escrowAddr.String(), + Msg: approveMsg, + Send: []wasmTypes.Coin{{ + Denom: "denom", + Amount: "14000", + }}, + }, + } + reflectSend := MaskHandleMsg{ + Reflect: &reflectPayload{ + Msg: msg, + }, + } + reflectSendBz, err := json.Marshal(reflectSend) + require.NoError(t, err) + _, err = keeper.Execute(ctx, maskAddr, creator, reflectSendBz, nil) + require.NoError(t, err) + + // did this work??? + checkAccount(t, ctx, accKeeper, creator, sdk.NewCoins(sdk.NewInt64Coin("denom", 35000))) // same as before + checkAccount(t, ctx, accKeeper, maskAddr, sdk.NewCoins(sdk.NewInt64Coin("denom", 26000))) // 40k - 14k (from send) + checkAccount(t, ctx, accKeeper, escrowAddr, sdk.Coins{}) // emptied reserved + checkAccount(t, ctx, accKeeper, bob, sdk.NewCoins(sdk.NewInt64Coin("denom", 39000))) // all escrow of 25k + 14k + +} + func checkAccount(t *testing.T, ctx sdk.Context, accKeeper auth.AccountKeeper, addr sdk.AccAddress, expected sdk.Coins) { acct := accKeeper.GetAccount(ctx, addr) if expected == nil { - require.Nil(t, acct) + assert.Nil(t, acct) } else { - require.NotNil(t, acct) - require.Equal(t, acct.GetCoins(), expected) + assert.NotNil(t, acct) + assert.Equal(t, acct.GetCoins(), expected) } } From e28db84189e08a7c867116270dea70265ef4298a Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 21 Jan 2020 14:13:36 +0100 Subject: [PATCH 2/2] Contract-to-contract send works --- go.mod | 2 +- go.sum | 2 ++ x/wasm/internal/keeper/keeper.go | 49 ++++++++++++++++++----------- x/wasm/internal/keeper/mask_test.go | 7 ++++- 4 files changed, 39 insertions(+), 21 deletions(-) diff --git a/go.mod b/go.mod index 1a1c56b24..f56dfd9c8 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.13 require ( github.com/btcsuite/btcd v0.0.0-20190807005414-4063feeff79a // indirect - github.com/confio/go-cosmwasm v0.6.1 + github.com/confio/go-cosmwasm v0.6.2 github.com/cosmos/cosmos-sdk v0.34.4-0.20191114141721-d4c831e63ad3 github.com/cosmos/go-bip39 v0.0.0-20180819234021-555e2067c45d // indirect github.com/golang/mock v1.3.1 // indirect diff --git a/go.sum b/go.sum index fa2dc09f7..5c9100e5a 100644 --- a/go.sum +++ b/go.sum @@ -38,6 +38,8 @@ github.com/confio/go-cosmwasm v0.6.0 h1:6MsfozR4IWb+V9TgVhDoGvcEs0ItBCqHg4pGbMaf github.com/confio/go-cosmwasm v0.6.0/go.mod h1:pHipRby+f3cv97QPLELkzOAlNs/s87uDyhc+SnMn7L4= github.com/confio/go-cosmwasm v0.6.1 h1:ifjLWE4T0mKngoq+ubZdtfGJFNWyOeQpfJWNO9y6WKI= github.com/confio/go-cosmwasm v0.6.1/go.mod h1:pHipRby+f3cv97QPLELkzOAlNs/s87uDyhc+SnMn7L4= +github.com/confio/go-cosmwasm v0.6.2 h1:UMi8mP6VjID1QMPnLMW6xP6zdn7hXmgQTOYSwQgCN0k= +github.com/confio/go-cosmwasm v0.6.2/go.mod h1:pHipRby+f3cv97QPLELkzOAlNs/s87uDyhc+SnMn7L4= github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk= github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= github.com/coreos/go-etcd v2.0.0+incompatible/go.mod h1:Jez6KQU2B/sWsbdaef3ED8NzMklzPG4d5KIOhIy30Tk= diff --git a/x/wasm/internal/keeper/keeper.go b/x/wasm/internal/keeper/keeper.go index 8ef371a29..fb495c0d7 100644 --- a/x/wasm/internal/keeper/keeper.go +++ b/x/wasm/internal/keeper/keeper.go @@ -296,43 +296,55 @@ func (k Keeper) dispatchMessages(ctx sdk.Context, contract exported.Account, msg } func (k Keeper) dispatchMessage(ctx sdk.Context, contract exported.Account, msg wasmTypes.CosmosMsg) error { - // we check each type (pointers would make it easier to test if set) + // maybe use this instead for the arg? + contractAddr := contract.GetAddress() if msg.Send != nil { - sendMsg, err := convertCosmosSendMsg(msg.Send) - if err != nil { - return err - } - return k.handleSdkMessage(ctx, contract, sendMsg) + return k.sendTokens(ctx, contractAddr, msg.Send.FromAddress, msg.Send.ToAddress, msg.Send.Amount) } else if msg.Contract != nil { targetAddr, stderr := sdk.AccAddressFromBech32(msg.Contract.ContractAddr) if stderr != nil { return sdk.ErrInvalidAddress(msg.Contract.ContractAddr) } - _, err := k.Execute(ctx, targetAddr, contract.GetAddress(), []byte(msg.Contract.Msg), nil) + err := k.sendTokens(ctx, contractAddr, contractAddr.String(), targetAddr.String(), msg.Contract.Send) + if err != nil { + return err + } + _, err = k.Execute(ctx, targetAddr, contractAddr, []byte(msg.Contract.Msg), nil) return err // may be nil } else if msg.Opaque != nil { msg, err := ParseOpaqueMsg(k.cdc, msg.Opaque) if err != nil { return err } - return k.handleSdkMessage(ctx, contract, msg) + return k.handleSdkMessage(ctx, contractAddr, msg) } // what is it? panic(fmt.Sprintf("Unknown CosmosMsg: %#v", msg)) } -func convertCosmosSendMsg(msg *wasmTypes.SendMsg) (bank.MsgSend, sdk.Error) { - fromAddr, stderr := sdk.AccAddressFromBech32(msg.FromAddress) +func (k Keeper) sendTokens(ctx sdk.Context, signer sdk.AccAddress, origin string, target string, tokens []wasmTypes.Coin) error { + if len(tokens) == 0 { + return nil + } + msg, err := convertCosmosSendMsg(origin, target, tokens) + if err != nil { + return err + } + return k.handleSdkMessage(ctx, signer, msg) +} + +func convertCosmosSendMsg(from string, to string, coins []wasmTypes.Coin) (bank.MsgSend, sdk.Error) { + fromAddr, stderr := sdk.AccAddressFromBech32(from) if stderr != nil { - return bank.MsgSend{}, sdk.ErrInvalidAddress(msg.FromAddress) + return bank.MsgSend{}, sdk.ErrInvalidAddress(from) } - toAddr, stderr := sdk.AccAddressFromBech32(msg.ToAddress) + toAddr, stderr := sdk.AccAddressFromBech32(to) if stderr != nil { - return bank.MsgSend{}, sdk.ErrInvalidAddress(msg.ToAddress) + return bank.MsgSend{}, sdk.ErrInvalidAddress(to) } - var coins sdk.Coins - for _, coin := range msg.Amount { + var toSend sdk.Coins + for _, coin := range coins { amount, ok := sdk.NewIntFromString(coin.Amount) if !ok { return bank.MsgSend{}, sdk.ErrInvalidCoins(coin.Amount + coin.Denom) @@ -341,19 +353,18 @@ func convertCosmosSendMsg(msg *wasmTypes.SendMsg) (bank.MsgSend, sdk.Error) { Denom: coin.Denom, Amount: amount, } - coins = append(coins, c) + toSend = append(toSend, c) } sendMsg := bank.MsgSend{ FromAddress: fromAddr, ToAddress: toAddr, - Amount: coins, + Amount: toSend, } return sendMsg, nil } -func (k Keeper) handleSdkMessage(ctx sdk.Context, contract exported.Account, msg sdk.Msg) error { +func (k Keeper) handleSdkMessage(ctx sdk.Context, contractAddr sdk.Address, msg sdk.Msg) error { // make sure this account can send it - contractAddr := contract.GetAddress() for _, acct := range msg.GetSigners() { if !acct.Equals(contractAddr) { return sdkErrors.Wrap(sdkErrors.ErrUnauthorized, "contract doesn't have permission") diff --git a/x/wasm/internal/keeper/mask_test.go b/x/wasm/internal/keeper/mask_test.go index 64aae4638..fd6c4355a 100644 --- a/x/wasm/internal/keeper/mask_test.go +++ b/x/wasm/internal/keeper/mask_test.go @@ -212,6 +212,11 @@ func checkAccount(t *testing.T, ctx sdk.Context, accKeeper auth.AccountKeeper, a assert.Nil(t, acct) } else { assert.NotNil(t, acct) - assert.Equal(t, acct.GetCoins(), expected) + if expected.Empty() { + // there is confusion between nil and empty slice... let's just treat them the same + assert.True(t, acct.GetCoins().Empty()) + } else { + assert.Equal(t, acct.GetCoins(), expected) + } } }