From f8dbfd12f0b43ebd55d755fa43dac87455cfe33e Mon Sep 17 00:00:00 2001 From: Alex Peters Date: Thu, 16 Jul 2020 13:14:23 +0200 Subject: [PATCH 1/4] Store code history for contract --- x/wasm/alias.go | 2 +- x/wasm/internal/keeper/genesis_test.go | 28 +++++---- x/wasm/internal/keeper/keeper.go | 4 +- x/wasm/internal/keeper/keeper_test.go | 53 +++++++++++----- .../keeper/proposal_integration_test.go | 23 ++++++- x/wasm/internal/keeper/querier.go | 19 +++--- x/wasm/internal/keeper/querier_test.go | 5 +- x/wasm/internal/keeper/test_fuzz.go | 27 ++++++-- x/wasm/internal/types/types.go | 62 +++++++++++++------ x/wasm/internal/types/types_test.go | 9 --- 10 files changed, 157 insertions(+), 75 deletions(-) diff --git a/x/wasm/alias.go b/x/wasm/alias.go index 354c9a00ca..7bb2362123 100644 --- a/x/wasm/alias.go +++ b/x/wasm/alias.go @@ -43,7 +43,7 @@ var ( GetContractAddressKey = types.GetContractAddressKey GetContractStorePrefixKey = types.GetContractStorePrefixKey NewCodeInfo = types.NewCodeInfo - NewCreatedAt = types.NewCreatedAt + NewCreatedAt = types.NewAbsoluteTxPosition NewContractInfo = types.NewContractInfo NewEnv = types.NewEnv NewWasmCoins = types.NewWasmCoins diff --git a/x/wasm/internal/keeper/genesis_test.go b/x/wasm/internal/keeper/genesis_test.go index 2dc5dccfc5..6e07f66ef9 100644 --- a/x/wasm/internal/keeper/genesis_test.go +++ b/x/wasm/internal/keeper/genesis_test.go @@ -2,6 +2,7 @@ package keeper import ( "crypto/sha256" + "encoding/json" "io/ioutil" "math/rand" "os" @@ -29,8 +30,8 @@ func TestGenesisExportImport(t *testing.T) { require.NoError(t, err) // store some test data - f := fuzz.New().Funcs(FuzzAddr, FuzzAbsoluteTxPosition, FuzzContractInfo, FuzzStateModel, FuzzAccessType, FuzzAccessConfig) - for i := 0; i < 25; i++ { + f := fuzz.New().Funcs(ModelFuzzers...) + for i := 0; i < 1; i++ { var ( codeInfo types.CodeInfo contract types.ContractInfo @@ -39,7 +40,6 @@ func TestGenesisExportImport(t *testing.T) { f.Fuzz(&codeInfo) f.Fuzz(&contract) f.Fuzz(&stateModels) - codeID, err := srcKeeper.Create(srcCtx, codeInfo.Creator, wasmCode, codeInfo.Source, codeInfo.Builder, &codeInfo.InstantiateConfig) require.NoError(t, err) contract.CodeID = codeID @@ -52,24 +52,28 @@ func TestGenesisExportImport(t *testing.T) { srcKeeper.setParams(srcCtx, wasmParams) // export - genesisState := ExportGenesis(srcCtx, srcKeeper) - + exportedState := ExportGenesis(srcCtx, srcKeeper) // order should not matter - rand.Shuffle(len(genesisState.Codes), func(i, j int) { - genesisState.Codes[i], genesisState.Codes[j] = genesisState.Codes[j], genesisState.Codes[i] + rand.Shuffle(len(exportedState.Codes), func(i, j int) { + exportedState.Codes[i], exportedState.Codes[j] = exportedState.Codes[j], exportedState.Codes[i] }) - rand.Shuffle(len(genesisState.Contracts), func(i, j int) { - genesisState.Contracts[i], genesisState.Contracts[j] = genesisState.Contracts[j], genesisState.Contracts[i] + rand.Shuffle(len(exportedState.Contracts), func(i, j int) { + exportedState.Contracts[i], exportedState.Contracts[j] = exportedState.Contracts[j], exportedState.Contracts[i] }) - rand.Shuffle(len(genesisState.Sequences), func(i, j int) { - genesisState.Sequences[i], genesisState.Sequences[j] = genesisState.Sequences[j], genesisState.Sequences[i] + rand.Shuffle(len(exportedState.Sequences), func(i, j int) { + exportedState.Sequences[i], exportedState.Sequences[j] = exportedState.Sequences[j], exportedState.Sequences[i] }) + exportedGenesis, err := json.Marshal(exportedState) + require.NoError(t, err) // re-import dstKeeper, dstCtx, dstStoreKeys, dstCleanup := setupKeeper(t) defer dstCleanup() - InitGenesis(dstCtx, dstKeeper, genesisState) + var importState wasmTypes.GenesisState + err = json.Unmarshal(exportedGenesis, &importState) + require.NoError(t, err) + InitGenesis(dstCtx, dstKeeper, importState) // compare whole DB for j := range srcStoreKeys { diff --git a/x/wasm/internal/keeper/keeper.go b/x/wasm/internal/keeper/keeper.go index 3f72549769..0ba8d38563 100644 --- a/x/wasm/internal/keeper/keeper.go +++ b/x/wasm/internal/keeper/keeper.go @@ -240,7 +240,7 @@ func (k Keeper) instantiate(ctx sdk.Context, codeID uint64, creator, admin sdk.A } // persist instance - createdAt := types.NewCreatedAt(ctx) + createdAt := types.NewAbsoluteTxPosition(ctx) instance := types.NewContractInfo(codeID, creator, admin, initMsg, label, createdAt) store.Set(types.GetContractAddressKey(contractAddress), k.cdc.MustMarshalBinaryBare(instance)) @@ -340,7 +340,7 @@ func (k Keeper) migrate(ctx sdk.Context, contractAddress sdk.AccAddress, caller events := types.ParseEvents(res.Log, contractAddress) ctx.EventManager().EmitEvents(events) - contractInfo.UpdateCodeID(ctx, newCodeID) + contractInfo.AddMigration(ctx, newCodeID, msg) k.setContractInfo(ctx, contractAddress, contractInfo) if err := k.dispatchMessages(ctx, contractAddress, res.Messages); err != nil { diff --git a/x/wasm/internal/keeper/keeper_test.go b/x/wasm/internal/keeper/keeper_test.go index 4771069505..84be249c9e 100644 --- a/x/wasm/internal/keeper/keeper_test.go +++ b/x/wasm/internal/keeper/keeper_test.go @@ -306,15 +306,23 @@ func TestInstantiate(t *testing.T) { require.Equal(t, "cosmos18vd8fpwxzck93qlwghaj6arh4p7c5n89uzcee5", addr.String()) gasAfter := ctx.GasMeter().GasConsumed() - require.Equal(t, uint64(0x1155d), gasAfter-gasBefore) + require.Equal(t, uint64(0x11740), gasAfter-gasBefore) // ensure it is stored properly info := keeper.GetContractInfo(ctx, addr) require.NotNil(t, info) assert.Equal(t, info.Creator, creator) assert.Equal(t, info.CodeID, contractID) - assert.Equal(t, info.InitMsg, json.RawMessage(initMsgBz)) assert.Equal(t, info.Label, "demo contract 1") + + exp := []types.ContractCodeHistoryEntry{{ + Operation: types.InitContractCodeHistoryType, + CodeID: contractID, + Updated: types.NewAbsoluteTxPosition(ctx), + Msg: json.RawMessage(initMsgBz), + }} + assert.Equal(t, exp, info.ContractCodeHistory) + } func TestInstantiateWithDeposit(t *testing.T) { @@ -527,7 +535,7 @@ func TestExecute(t *testing.T) { // make sure gas is properly deducted from ctx gasAfter := ctx.GasMeter().GasConsumed() - require.Equal(t, uint64(0x11c16), gasAfter-gasBefore) + require.Equal(t, uint64(0x11c2e), gasAfter-gasBefore) // ensure bob now exists and got both payments released bobAcct = accKeeper.GetAccount(ctx, bob) @@ -772,11 +780,11 @@ func TestMigrate(t *testing.T) { wasmCode, err := ioutil.ReadFile("./testdata/contract.wasm") require.NoError(t, err) - originalContractID, err := keeper.Create(ctx, creator, wasmCode, "", "", nil) + originalCodeID, err := keeper.Create(ctx, creator, wasmCode, "", "", nil) require.NoError(t, err) - newContractID, err := keeper.Create(ctx, creator, wasmCode, "", "", nil) + newCodeID, err := keeper.Create(ctx, creator, wasmCode, "", "", nil) require.NoError(t, err) - require.NotEqual(t, originalContractID, newContractID) + require.NotEqual(t, originalCodeID, newCodeID) _, _, anyAddr := keyPubAddr() _, _, newVerifierAddr := keyPubAddr() @@ -805,33 +813,33 @@ func TestMigrate(t *testing.T) { "all good with same code id": { admin: creator, caller: creator, - codeID: originalContractID, + codeID: originalCodeID, migrateMsg: migMsgBz, expVerifier: newVerifierAddr, }, "all good with different code id": { admin: creator, caller: creator, - codeID: newContractID, + codeID: newCodeID, migrateMsg: migMsgBz, expVerifier: newVerifierAddr, }, "all good with admin set": { admin: fred, caller: fred, - codeID: newContractID, + codeID: newCodeID, migrateMsg: migMsgBz, expVerifier: newVerifierAddr, }, "prevent migration when admin was not set on instantiate": { caller: creator, - codeID: originalContractID, + codeID: originalCodeID, expErr: sdkerrors.ErrUnauthorized, }, "prevent migration when not sent by admin": { caller: creator, admin: fred, - codeID: originalContractID, + codeID: originalCodeID, expErr: sdkerrors.ErrUnauthorized, }, "fail with non existing code id": { @@ -844,20 +852,20 @@ func TestMigrate(t *testing.T) { admin: creator, caller: creator, overrideContractAddr: anyAddr, - codeID: originalContractID, + codeID: originalCodeID, expErr: sdkerrors.ErrInvalidRequest, }, "fail in contract with invalid migrate msg": { admin: creator, caller: creator, - codeID: originalContractID, + codeID: originalCodeID, migrateMsg: bytes.Repeat([]byte{0x1}, 7), expErr: types.ErrMigrationFailed, }, "fail in contract without migrate msg": { admin: creator, caller: creator, - codeID: originalContractID, + codeID: originalCodeID, expErr: types.ErrMigrationFailed, }, } @@ -865,7 +873,7 @@ func TestMigrate(t *testing.T) { for msg, spec := range specs { t.Run(msg, func(t *testing.T) { ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) - addr, err := keeper.Instantiate(ctx, originalContractID, creator, spec.admin, initMsgBz, "demo contract", nil) + addr, err := keeper.Instantiate(ctx, originalCodeID, creator, spec.admin, initMsgBz, "demo contract", nil) require.NoError(t, err) if spec.overrideContractAddr != nil { addr = spec.overrideContractAddr @@ -877,8 +885,19 @@ func TestMigrate(t *testing.T) { } cInfo := keeper.GetContractInfo(ctx, addr) assert.Equal(t, spec.codeID, cInfo.CodeID) - assert.Equal(t, originalContractID, cInfo.PreviousCodeID) - assert.Equal(t, types.NewCreatedAt(ctx), cInfo.LastUpdated) + + expHistory := []types.ContractCodeHistoryEntry{{ + Operation: types.InitContractCodeHistoryType, + CodeID: originalCodeID, + Updated: types.NewAbsoluteTxPosition(ctx), + Msg: initMsgBz, + }, { + Operation: types.MigrateContractCodeHistoryType, + CodeID: spec.codeID, + Updated: types.NewAbsoluteTxPosition(ctx), + Msg: spec.migrateMsg, + }} + assert.Equal(t, expHistory, cInfo.ContractCodeHistory) m := keeper.QueryRaw(ctx, addr, []byte("config")) require.Len(t, m, 1) diff --git a/x/wasm/internal/keeper/proposal_integration_test.go b/x/wasm/internal/keeper/proposal_integration_test.go index 9feee2dc73..c4e36a11f8 100644 --- a/x/wasm/internal/keeper/proposal_integration_test.go +++ b/x/wasm/internal/keeper/proposal_integration_test.go @@ -104,7 +104,14 @@ func TestInstantiateProposal(t *testing.T) { assert.Equal(t, oneAddress, cInfo.Creator) assert.Equal(t, otherAddress, cInfo.Admin) assert.Equal(t, "testing", cInfo.Label) - assert.Equal(t, src.InitMsg, cInfo.InitMsg) + expHistory := []types.ContractCodeHistoryEntry{{ + Operation: types.InitContractCodeHistoryType, + CodeID: src.Code, + Updated: types.NewAbsoluteTxPosition(ctx), + Msg: src.InitMsg, + }} + assert.Equal(t, expHistory, cInfo.ContractCodeHistory) + } func TestMigrateProposal(t *testing.T) { @@ -169,9 +176,21 @@ func TestMigrateProposal(t *testing.T) { cInfo := wasmKeeper.GetContractInfo(ctx, contractAddr) require.NotNil(t, cInfo) assert.Equal(t, uint64(2), cInfo.CodeID) - assert.Equal(t, uint64(1), cInfo.PreviousCodeID) assert.Equal(t, anyAddress, cInfo.Admin) assert.Equal(t, "testing", cInfo.Label) + expHistory := []types.ContractCodeHistoryEntry{{ + // Operation: types.InitContractCodeHistoryType, + // CodeID: 1, + // Updated: types.NewAbsoluteTxPosition(ctx), + // Msg: initMsgBz, + //}, { + Operation: types.MigrateContractCodeHistoryType, + CodeID: src.Code, + Updated: types.NewAbsoluteTxPosition(ctx), + Msg: src.MigrateMsg, + }} + assert.Equal(t, expHistory, cInfo.ContractCodeHistory) + } func TestAdminProposals(t *testing.T) { diff --git a/x/wasm/internal/keeper/querier.go b/x/wasm/internal/keeper/querier.go index 6a6b799d38..8881a8cc20 100644 --- a/x/wasm/internal/keeper/querier.go +++ b/x/wasm/internal/keeper/querier.go @@ -69,11 +69,7 @@ func queryContractInfo(ctx sdk.Context, bech string, req abci.RequestQuery, keep if info == nil { return []byte("null"), nil } - // redact the Created field (just used for sorting, not part of public API) - info.Created = nil - info.LastUpdated = nil - info.PreviousCodeID = 0 - + redact(info) infoWithAddress := ContractInfoWithAddress{ Address: addr, ContractInfo: info, @@ -85,6 +81,15 @@ func queryContractInfo(ctx sdk.Context, bech string, req abci.RequestQuery, keep return bz, nil } +// redact clears all fields not in the public api +func redact(info *types.ContractInfo) { + info.Created = nil + for i := range info.ContractCodeHistory { + info.ContractCodeHistory[i].Updated = nil + info.ContractCodeHistory[i].Msg = nil + } +} + func queryContractListByCode(ctx sdk.Context, codeIDstr string, req abci.RequestQuery, keeper Keeper) ([]byte, error) { codeID, err := strconv.ParseUint(codeIDstr, 10, 64) if err != nil { @@ -94,8 +99,6 @@ func queryContractListByCode(ctx sdk.Context, codeIDstr string, req abci.Request var contracts []ContractInfoWithAddress keeper.ListContractInfo(ctx, func(addr sdk.AccAddress, info types.ContractInfo) bool { if info.CodeID == codeID { - // remove init message on list - info.InitMsg = nil // and add the address infoWithAddress := ContractInfoWithAddress{ Address: addr, @@ -112,7 +115,7 @@ func queryContractListByCode(ctx sdk.Context, codeIDstr string, req abci.Request }) // and remove that info for the final json (yes, the json:"-" tag doesn't work) for i := range contracts { - contracts[i].Created = nil + redact(contracts[i].ContractInfo) } bz, err := json.MarshalIndent(contracts, "", " ") diff --git a/x/wasm/internal/keeper/querier_test.go b/x/wasm/internal/keeper/querier_test.go index 2b6ebcbee9..ef1bc7957b 100644 --- a/x/wasm/internal/keeper/querier_test.go +++ b/x/wasm/internal/keeper/querier_test.go @@ -208,7 +208,10 @@ func TestListContractByCodeOrdering(t *testing.T) { assert.Equal(t, fmt.Sprintf("contract %d", i), contract.Label) assert.NotEmpty(t, contract.Address) // ensure these are not shown - assert.Nil(t, contract.InitMsg) assert.Nil(t, contract.Created) + for _, entry := range contract.ContractCodeHistory { + assert.Nil(t, entry.Updated) + assert.Nil(t, entry.Msg) + } } } diff --git a/x/wasm/internal/keeper/test_fuzz.go b/x/wasm/internal/keeper/test_fuzz.go index cc83c8670a..ffc3694ea4 100644 --- a/x/wasm/internal/keeper/test_fuzz.go +++ b/x/wasm/internal/keeper/test_fuzz.go @@ -1,12 +1,16 @@ package keeper import ( + "encoding/json" + "github.com/CosmWasm/wasmd/x/wasm/internal/types" sdk "github.com/cosmos/cosmos-sdk/types" fuzz "github.com/google/gofuzz" tmBytes "github.com/tendermint/tendermint/libs/bytes" ) +var ModelFuzzers = []interface{}{FuzzAddr, FuzzAbsoluteTxPosition, FuzzContractInfo, FuzzStateModel, FuzzAccessType, FuzzAccessConfig, FuzzContractCodeHistory} + func FuzzAddr(m *sdk.AccAddress, c fuzz.Continue) { *m = make([]byte, 20) c.Read(*m) @@ -18,16 +22,29 @@ func FuzzAbsoluteTxPosition(m *types.AbsoluteTxPosition, c fuzz.Continue) { } func FuzzContractInfo(m *types.ContractInfo, c fuzz.Continue) { - const maxSize = 1024 m.CodeID = c.RandUint64() FuzzAddr(&m.Creator, c) FuzzAddr(&m.Admin, c) m.Label = c.RandString() - m.InitMsg = make([]byte, c.RandUint64()%maxSize) - c.Read(m.InitMsg) c.Fuzz(&m.Created) - c.Fuzz(&m.LastUpdated) - m.PreviousCodeID = c.RandUint64() + historyElements := c.Int() % 128 // 128 should be enough for tests + m.ContractCodeHistory = make([]types.ContractCodeHistoryEntry, historyElements) + for i := range m.ContractCodeHistory { + c.Fuzz(&m.ContractCodeHistory[i]) + } +} + +func FuzzContractCodeHistory(m *types.ContractCodeHistoryEntry, c fuzz.Continue) { + const maxMsgSize = 128 + m.CodeID = c.RandUint64() + msg := make([]byte, c.RandUint64()%maxMsgSize) + c.Read(msg) + var err error + if m.Msg, err = json.Marshal(msg); err != nil { + panic(err) + } + c.Fuzz(&m.Updated) + m.Operation = types.AllCodeHistoryTypes[c.Int()%len(types.AllCodeHistoryTypes)] } func FuzzStateModel(m *types.Model, c fuzz.Continue) { diff --git a/x/wasm/internal/types/types.go b/x/wasm/internal/types/types.go index 16e91a6e05..4b9178950a 100644 --- a/x/wasm/internal/types/types.go +++ b/x/wasm/internal/types/types.go @@ -2,6 +2,7 @@ package types import ( "encoding/json" + "sort" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" tmBytes "github.com/tendermint/tendermint/libs/bytes" @@ -64,24 +65,46 @@ func NewCodeInfo(codeHash []byte, creator sdk.AccAddress, source string, builder } } +type ContractCodeHistoryOperationType string + +const ( + InitContractCodeHistoryType ContractCodeHistoryOperationType = "Init" + MigrateContractCodeHistoryType ContractCodeHistoryOperationType = "Migrate" +) + +var AllCodeHistoryTypes = []ContractCodeHistoryOperationType{InitContractCodeHistoryType, MigrateContractCodeHistoryType} + +type ContractCodeHistoryEntry struct { + Operation ContractCodeHistoryOperationType `json:"operation"` + CodeID uint64 `json:"code_id"` + Updated *AbsoluteTxPosition `json:"updated,omitempty"` + Msg json.RawMessage `json:"msg,omitempty"` +} + // ContractInfo stores a WASM contract instance type ContractInfo struct { - CodeID uint64 `json:"code_id"` - Creator sdk.AccAddress `json:"creator"` - Admin sdk.AccAddress `json:"admin,omitempty"` - Label string `json:"label"` - InitMsg json.RawMessage `json:"init_msg,omitempty"` + CodeID uint64 `json:"code_id"` + Creator sdk.AccAddress `json:"creator"` + Admin sdk.AccAddress `json:"admin,omitempty"` + Label string `json:"label"` // never show this in query results, just use for sorting // (Note: when using json tag "-" amino refused to serialize it...) - Created *AbsoluteTxPosition `json:"created,omitempty"` - LastUpdated *AbsoluteTxPosition `json:"last_updated,omitempty"` - PreviousCodeID uint64 `json:"previous_code_id,omitempty"` + Created *AbsoluteTxPosition `json:"created,omitempty"` + ContractCodeHistory []ContractCodeHistoryEntry `json:"contract_code_history"` } -func (c *ContractInfo) UpdateCodeID(ctx sdk.Context, newCodeID uint64) { - c.PreviousCodeID = c.CodeID - c.CodeID = newCodeID - c.LastUpdated = NewCreatedAt(ctx) +func (c *ContractInfo) AddMigration(ctx sdk.Context, codeID uint64, msg []byte) { + h := ContractCodeHistoryEntry{ + Operation: MigrateContractCodeHistoryType, + CodeID: codeID, + Updated: NewAbsoluteTxPosition(ctx), + Msg: msg, + } + c.ContractCodeHistory = append(c.ContractCodeHistory, h) + sort.Slice(c.ContractCodeHistory, func(i, j int) bool { + return c.ContractCodeHistory[i].Updated.LessThan(c.ContractCodeHistory[j].Updated) + }) + c.CodeID = codeID } func (c *ContractInfo) ValidateBasic() error { @@ -105,9 +128,7 @@ func (c *ContractInfo) ValidateBasic() error { if err := c.Created.ValidateBasic(); err != nil { return sdkerrors.Wrap(err, "created") } - if err := c.LastUpdated.ValidateBasic(); err != nil { - return sdkerrors.Wrap(err, "last updated") - } + // TODO: validate history return nil } @@ -140,8 +161,8 @@ func (a *AbsoluteTxPosition) ValidateBasic() error { return nil } -// NewCreatedAt gets a timestamp from the context -func NewCreatedAt(ctx sdk.Context) *AbsoluteTxPosition { +// NewAbsoluteTxPosition gets a timestamp from the context +func NewAbsoluteTxPosition(ctx sdk.Context) *AbsoluteTxPosition { // we must safely handle nil gas meters var index uint64 meter := ctx.BlockGasMeter() @@ -160,9 +181,14 @@ func NewContractInfo(codeID uint64, creator, admin sdk.AccAddress, initMsg []byt CodeID: codeID, Creator: creator, Admin: admin, - InitMsg: initMsg, Label: label, Created: createdAt, + ContractCodeHistory: []ContractCodeHistoryEntry{{ + Operation: InitContractCodeHistoryType, + CodeID: codeID, + Updated: createdAt, + Msg: initMsg, + }}, } } diff --git a/x/wasm/internal/types/types_test.go b/x/wasm/internal/types/types_test.go index 07498a0f98..04e84e302e 100644 --- a/x/wasm/internal/types/types_test.go +++ b/x/wasm/internal/types/types_test.go @@ -42,9 +42,6 @@ func TestContractInfoValidateBasic(t *testing.T) { srcMutator: func(c *ContractInfo) { c.Label = strings.Repeat("a", MaxLabelSize+1) }, expError: true, }, - "init msg empty": { - srcMutator: func(c *ContractInfo) { c.InitMsg = nil }, - }, "created nil": { srcMutator: func(c *ContractInfo) { c.Created = nil }, expError: true, @@ -53,12 +50,6 @@ func TestContractInfoValidateBasic(t *testing.T) { srcMutator: func(c *ContractInfo) { c.Created = &AbsoluteTxPosition{BlockHeight: -1} }, expError: true, }, - "last updated nil": { - srcMutator: func(c *ContractInfo) { c.LastUpdated = nil }, - }, - "previous code id empty": { - srcMutator: func(c *ContractInfo) { c.PreviousCodeID = 0 }, - }, } for msg, spec := range specs { t.Run(msg, func(t *testing.T) { From 871896c62ba519db24276b4a298bacaebeb30e57 Mon Sep 17 00:00:00 2001 From: Alex Peters Date: Thu, 16 Jul 2020 16:45:59 +0200 Subject: [PATCH 2/4] Reset history on import --- CHANGELOG.md | 1 + x/wasm/alias.go | 2 +- x/wasm/internal/keeper/genesis.go | 3 + x/wasm/internal/keeper/genesis_test.go | 138 ++++++++++++++++-- x/wasm/internal/keeper/keeper.go | 2 + x/wasm/internal/keeper/keeper_test.go | 4 +- .../keeper/proposal_integration_test.go | 9 +- x/wasm/internal/types/genesis.go | 7 + x/wasm/internal/types/genesis_test.go | 12 ++ x/wasm/internal/types/test_fixtures.go | 7 +- x/wasm/internal/types/types.go | 21 ++- x/wasm/internal/types/types_test.go | 8 - 12 files changed, 180 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37bbf5f0d8..a922881ce1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] ### Features +* (wasmd) [\#130](https://github.com/CosmWasm/wasmd/issues/130) Full history of contract code migrations * (wasmd) [\#187](https://github.com/CosmWasm/wasmd/issues/187) Introduce wasmgovd binary * (wasmd) [\#178](https://github.com/CosmWasm/wasmd/issues/178) Add cli support for wasm gov proposals * (wasmd) [\#163](https://github.com/CosmWasm/wasmd/issues/163) Control who can instantiate code diff --git a/x/wasm/alias.go b/x/wasm/alias.go index 7bb2362123..e0755f2acf 100644 --- a/x/wasm/alias.go +++ b/x/wasm/alias.go @@ -43,7 +43,7 @@ var ( GetContractAddressKey = types.GetContractAddressKey GetContractStorePrefixKey = types.GetContractStorePrefixKey NewCodeInfo = types.NewCodeInfo - NewCreatedAt = types.NewAbsoluteTxPosition + NewAbsoluteTxPosition = types.NewAbsoluteTxPosition NewContractInfo = types.NewContractInfo NewEnv = types.NewEnv NewWasmCoins = types.NewWasmCoins diff --git a/x/wasm/internal/keeper/genesis.go b/x/wasm/internal/keeper/genesis.go index a84038377c..fa7402def4 100644 --- a/x/wasm/internal/keeper/genesis.go +++ b/x/wasm/internal/keeper/genesis.go @@ -83,6 +83,9 @@ func ExportGenesis(ctx sdk.Context, keeper Keeper) types.GenesisState { } state = append(state, m) } + // redact contract info + contract.Created = nil + contract.ContractCodeHistory = nil genState.Contracts = append(genState.Contracts, types.Contract{ ContractAddress: addr, diff --git a/x/wasm/internal/keeper/genesis_test.go b/x/wasm/internal/keeper/genesis_test.go index 6e07f66ef9..d4544bb773 100644 --- a/x/wasm/internal/keeper/genesis_test.go +++ b/x/wasm/internal/keeper/genesis_test.go @@ -2,7 +2,9 @@ package keeper import ( "crypto/sha256" + "encoding/base64" "encoding/json" + "fmt" "io/ioutil" "math/rand" "os" @@ -17,6 +19,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/params" "github.com/cosmos/cosmos-sdk/x/staking" fuzz "github.com/google/gofuzz" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/libs/log" @@ -31,7 +34,7 @@ func TestGenesisExportImport(t *testing.T) { // store some test data f := fuzz.New().Funcs(ModelFuzzers...) - for i := 0; i < 1; i++ { + for i := 0; i < 25; i++ { var ( codeInfo types.CodeInfo contract types.ContractInfo @@ -66,6 +69,13 @@ func TestGenesisExportImport(t *testing.T) { exportedGenesis, err := json.Marshal(exportedState) require.NoError(t, err) + // reset contract history in source DB for comparision with dest DB + srcKeeper.ListContractInfo(srcCtx, func(address sdk.AccAddress, info wasmTypes.ContractInfo) bool { + info.ResetFromGenesis(srcCtx) + srcKeeper.setContractInfo(srcCtx, address, &info) + return false + }) + // re-import dstKeeper, dstCtx, dstStoreKeys, dstCleanup := setupKeeper(t) defer dstCleanup() @@ -215,7 +225,7 @@ func TestFailFastImport(t *testing.T) { Contracts: []types.Contract{ { ContractAddress: contractAddress(1, 1), - ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }), + ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }, types.OnlyGenesisFields), }, }, Sequences: []types.Sequence{ @@ -239,10 +249,10 @@ func TestFailFastImport(t *testing.T) { Contracts: []types.Contract{ { ContractAddress: contractAddress(1, 1), - ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }), + ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }, types.OnlyGenesisFields), }, { ContractAddress: contractAddress(1, 2), - ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }), + ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }, types.OnlyGenesisFields), }, }, Sequences: []types.Sequence{ @@ -258,7 +268,7 @@ func TestFailFastImport(t *testing.T) { Contracts: []types.Contract{ { ContractAddress: contractAddress(1, 1), - ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }), + ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }, types.OnlyGenesisFields), }, }, Params: types.DefaultParams(), @@ -277,10 +287,10 @@ func TestFailFastImport(t *testing.T) { Contracts: []types.Contract{ { ContractAddress: contractAddress(1, 1), - ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }), + ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }, types.OnlyGenesisFields), }, { ContractAddress: contractAddress(1, 1), - ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }), + ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }, types.OnlyGenesisFields), }, }, Params: types.DefaultParams(), @@ -299,7 +309,7 @@ func TestFailFastImport(t *testing.T) { Contracts: []types.Contract{ { ContractAddress: contractAddress(1, 1), - ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }), + ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }, types.OnlyGenesisFields), ContractState: []types.Model{ { Key: []byte{0x1}, @@ -353,7 +363,7 @@ func TestFailFastImport(t *testing.T) { Contracts: []types.Contract{ { ContractAddress: contractAddress(1, 1), - ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }), + ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }, types.OnlyGenesisFields), }, }, Sequences: []types.Sequence{ @@ -381,6 +391,116 @@ func TestFailFastImport(t *testing.T) { } } +func TestExportShouldNotContainContractCodeHistory(t *testing.T) { + tempDir, err := ioutil.TempDir("", "wasm") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + ctx, keepers := CreateTestInput(t, false, tempDir, SupportedFeatures, nil, nil) + accKeeper, keeper := keepers.AccountKeeper, keepers.WasmKeeper + + wasmCode, err := ioutil.ReadFile("./testdata/contract.wasm") + require.NoError(t, err) + var ( + deposit = sdk.NewCoins(sdk.NewInt64Coin("denom", 100000)) + creator = createFakeFundedAccount(ctx, accKeeper, deposit) + anyAddr = make([]byte, sdk.AddrLen) + ) + + firstCodeID, err := keeper.Create(ctx, creator, wasmCode, "https://github.com/CosmWasm/wasmd/blob/master/x/wasm/testdata/escrow.wasm", "", &types.AllowEverybody) + require.NoError(t, err) + secondCodeID, err := keeper.Create(ctx, creator, wasmCode, "https://github.com/CosmWasm/wasmd/blob/master/x/wasm/testdata/escrow.wasm", "", &types.AllowEverybody) + require.NoError(t, err) + initMsg := InitMsg{ + Verifier: anyAddr, + Beneficiary: anyAddr, + } + initMsgBz, err := json.Marshal(initMsg) + require.NoError(t, err) + + // create instance + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + contractAddr, err := keeper.Instantiate(ctx, firstCodeID, creator, creator, initMsgBz, "demo contract 1", nil) + require.NoError(t, err) + + // and migrate to second code id + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + _, err = keeper.Migrate(ctx, contractAddr, creator, secondCodeID, initMsgBz) + require.NoError(t, err) + // and contract contains 2 history elements + contractInfo := keeper.GetContractInfo(ctx, contractAddr) + require.NotNil(t, contractInfo) + require.Len(t, contractInfo.ContractCodeHistory, 2) + // when exported + state := ExportGenesis(ctx, keeper) + require.NoError(t, state.ValidateBasic()) + require.Len(t, state.Contracts, 1) + assert.Len(t, state.Contracts[0].ContractInfo.ContractCodeHistory, 0) + assert.Nil(t, state.Contracts[0].ContractInfo.Created) +} + +func TestImportContractWithCodeHistoryReset(t *testing.T) { + genesis := ` +{ + "params":{ + "code_upload_access": { + "permission": "Everybody" + }, + "instantiate_default_permission": "Everybody" + }, + "codes": [ + { + "code_id": 1, + "code_info": { + "code_hash": "mrFpzGE5s+Qfn9Xe7OikXc0q169m7sMm4ZuV6pVA8Mc=", + "creator": "cosmos1qtu5n0cnhfkjj6l2rq97hmky9fd89gwca9yarx", + "source": "https://example.com", + "builder": "foo/bar:tag", + "instantiate_config": { + "type": 1, + "address": "" + } + }, + "code_bytes": %q + } + ], + "contracts": [ + { + "contract_address": "cosmos18vd8fpwxzck93qlwghaj6arh4p7c5n89uzcee5", + "contract_info": { + "code_id": 1, + "creator": "cosmos13x849jzd03vne42ynpj25hn8npjecxqrjghd8x", + "admin": "cosmos1h5t8zxmjr30e9dqghtlpl40f2zz5cgey6esxtn", + "label": "ȀĴnZV芢毤" + } + } + ] +}` + wasmCode, err := ioutil.ReadFile("./testdata/contract.wasm") + require.NoError(t, err) + + keeper, ctx, _, dstCleanup := setupKeeper(t) + defer dstCleanup() + + var importState wasmTypes.GenesisState + err = json.Unmarshal([]byte(fmt.Sprintf(genesis, base64.StdEncoding.EncodeToString(wasmCode))), &importState) + require.NoError(t, err) + require.NoError(t, importState.ValidateBasic()) + InitGenesis(ctx, keeper, importState) + + contractAddr, err := sdk.AccAddressFromBech32("cosmos18vd8fpwxzck93qlwghaj6arh4p7c5n89uzcee5") + require.NoError(t, err) + contractInfo := keeper.GetContractInfo(ctx, contractAddr) + require.NotNil(t, contractInfo) + require.Len(t, contractInfo.ContractCodeHistory, 1) + exp := []types.ContractCodeHistoryEntry{{ + Operation: types.GenesisContractCodeHistoryType, + CodeID: 1, + Updated: types.NewAbsoluteTxPosition(ctx), + }, + } + assert.Equal(t, exp, contractInfo.ContractCodeHistory) +} + func setupKeeper(t *testing.T) (Keeper, sdk.Context, []sdk.StoreKey, func()) { t.Helper() tempDir, err := ioutil.TempDir("", "wasm") diff --git a/x/wasm/internal/keeper/keeper.go b/x/wasm/internal/keeper/keeper.go index 0ba8d38563..d781a28d11 100644 --- a/x/wasm/internal/keeper/keeper.go +++ b/x/wasm/internal/keeper/keeper.go @@ -606,6 +606,8 @@ func (k Keeper) importContract(ctx sdk.Context, address sdk.AccAddress, c *types if k.containsContractInfo(ctx, address) { return errors.Wrapf(types.ErrDuplicate, "contract: %s", address) } + + c.ResetFromGenesis(ctx) k.setContractInfo(ctx, address, c) return k.importContractState(ctx, address, state) } diff --git a/x/wasm/internal/keeper/keeper_test.go b/x/wasm/internal/keeper/keeper_test.go index 84be249c9e..c4f341308c 100644 --- a/x/wasm/internal/keeper/keeper_test.go +++ b/x/wasm/internal/keeper/keeper_test.go @@ -306,7 +306,7 @@ func TestInstantiate(t *testing.T) { require.Equal(t, "cosmos18vd8fpwxzck93qlwghaj6arh4p7c5n89uzcee5", addr.String()) gasAfter := ctx.GasMeter().GasConsumed() - require.Equal(t, uint64(0x11740), gasAfter-gasBefore) + require.Equal(t, uint64(0x1175b), gasAfter-gasBefore) // ensure it is stored properly info := keeper.GetContractInfo(ctx, addr) @@ -535,7 +535,7 @@ func TestExecute(t *testing.T) { // make sure gas is properly deducted from ctx gasAfter := ctx.GasMeter().GasConsumed() - require.Equal(t, uint64(0x11c2e), gasAfter-gasBefore) + require.Equal(t, uint64(0x11c49), gasAfter-gasBefore) // ensure bob now exists and got both payments released bobAcct = accKeeper.GetAccount(ctx, bob) diff --git a/x/wasm/internal/keeper/proposal_integration_test.go b/x/wasm/internal/keeper/proposal_integration_test.go index c4e36a11f8..b8fde66ce7 100644 --- a/x/wasm/internal/keeper/proposal_integration_test.go +++ b/x/wasm/internal/keeper/proposal_integration_test.go @@ -179,11 +179,10 @@ func TestMigrateProposal(t *testing.T) { assert.Equal(t, anyAddress, cInfo.Admin) assert.Equal(t, "testing", cInfo.Label) expHistory := []types.ContractCodeHistoryEntry{{ - // Operation: types.InitContractCodeHistoryType, - // CodeID: 1, - // Updated: types.NewAbsoluteTxPosition(ctx), - // Msg: initMsgBz, - //}, { + Operation: types.GenesisContractCodeHistoryType, + CodeID: 1, + Updated: types.NewAbsoluteTxPosition(ctx), + }, { Operation: types.MigrateContractCodeHistoryType, CodeID: src.Code, Updated: types.NewAbsoluteTxPosition(ctx), diff --git a/x/wasm/internal/types/genesis.go b/x/wasm/internal/types/genesis.go index be6a70bf5c..1ca7b2842f 100644 --- a/x/wasm/internal/types/genesis.go +++ b/x/wasm/internal/types/genesis.go @@ -82,6 +82,13 @@ func (c Contract) ValidateBasic() error { if err := c.ContractInfo.ValidateBasic(); err != nil { return sdkerrors.Wrap(err, "contract info") } + + if c.ContractInfo.Created != nil { + return sdkerrors.Wrap(ErrInvalid, "created must be empty") + } + if len(c.ContractInfo.ContractCodeHistory) != 0 { + return sdkerrors.Wrap(ErrInvalid, "history must be empty") + } for i := range c.ContractState { if err := c.ContractState[i].ValidateBasic(); err != nil { return sdkerrors.Wrapf(err, "contract state %d", i) diff --git a/x/wasm/internal/types/genesis_test.go b/x/wasm/internal/types/genesis_test.go index a94e54fac1..f241e6a063 100644 --- a/x/wasm/internal/types/genesis_test.go +++ b/x/wasm/internal/types/genesis_test.go @@ -121,6 +121,18 @@ func TestContractValidateBasic(t *testing.T) { }, expError: true, }, + "contract with created set": { + srcMutator: func(c *Contract) { + c.ContractInfo.Created = &AbsoluteTxPosition{} + }, + expError: true, + }, + "contract with history set": { + srcMutator: func(c *Contract) { + c.ContractInfo.ContractCodeHistory = []ContractCodeHistoryEntry{{}} + }, + expError: true, + }, "contract state invalid": { srcMutator: func(c *Contract) { c.ContractState = append(c.ContractState, Model{}) diff --git a/x/wasm/internal/types/test_fixtures.go b/x/wasm/internal/types/test_fixtures.go index 555147377e..22dbfe12c2 100644 --- a/x/wasm/internal/types/test_fixtures.go +++ b/x/wasm/internal/types/test_fixtures.go @@ -80,7 +80,7 @@ func ContractFixture(mutators ...func(*Contract)) Contract { anyAddress := make([]byte, 20) fixture := Contract{ ContractAddress: anyAddress, - ContractInfo: ContractInfoFixture(), + ContractInfo: ContractInfoFixture(OnlyGenesisFields), ContractState: []Model{{Key: []byte("anyKey"), Value: []byte("anyValue")}}, } @@ -90,6 +90,11 @@ func ContractFixture(mutators ...func(*Contract)) Contract { return fixture } +func OnlyGenesisFields(info *ContractInfo) { + info.Created = nil + info.ContractCodeHistory = nil +} + func ContractInfoFixture(mutators ...func(*ContractInfo)) ContractInfo { anyAddress := make([]byte, 20) fixture := ContractInfo{ diff --git a/x/wasm/internal/types/types.go b/x/wasm/internal/types/types.go index 4b9178950a..c6d69f8286 100644 --- a/x/wasm/internal/types/types.go +++ b/x/wasm/internal/types/types.go @@ -70,6 +70,7 @@ type ContractCodeHistoryOperationType string const ( InitContractCodeHistoryType ContractCodeHistoryOperationType = "Init" MigrateContractCodeHistoryType ContractCodeHistoryOperationType = "Migrate" + GenesisContractCodeHistoryType ContractCodeHistoryOperationType = "Genesis" ) var AllCodeHistoryTypes = []ContractCodeHistoryOperationType{InitContractCodeHistoryType, MigrateContractCodeHistoryType} @@ -90,7 +91,7 @@ type ContractInfo struct { // never show this in query results, just use for sorting // (Note: when using json tag "-" amino refused to serialize it...) Created *AbsoluteTxPosition `json:"created,omitempty"` - ContractCodeHistory []ContractCodeHistoryEntry `json:"contract_code_history"` + ContractCodeHistory []ContractCodeHistoryEntry `json:"contract_code_history,omitempty"` } func (c *ContractInfo) AddMigration(ctx sdk.Context, codeID uint64, msg []byte) { @@ -122,16 +123,20 @@ func (c *ContractInfo) ValidateBasic() error { if err := validateLabel(c.Label); err != nil { return sdkerrors.Wrap(err, "label") } - if c.Created == nil { - return sdkerrors.Wrap(ErrEmpty, "created") - } - if err := c.Created.ValidateBasic(); err != nil { - return sdkerrors.Wrap(err, "created") - } - // TODO: validate history return nil } +// ResetFromGenesis resets contracts timestamp and history. +func (c *ContractInfo) ResetFromGenesis(ctx sdk.Context) { + c.Created = NewAbsoluteTxPosition(ctx) + h := ContractCodeHistoryEntry{ + Operation: GenesisContractCodeHistoryType, + CodeID: c.CodeID, + Updated: c.Created, + } + c.ContractCodeHistory = []ContractCodeHistoryEntry{h} +} + // AbsoluteTxPosition can be used to sort contracts type AbsoluteTxPosition struct { // BlockHeight is the block the contract was created at diff --git a/x/wasm/internal/types/types_test.go b/x/wasm/internal/types/types_test.go index 04e84e302e..ad63c476af 100644 --- a/x/wasm/internal/types/types_test.go +++ b/x/wasm/internal/types/types_test.go @@ -42,14 +42,6 @@ func TestContractInfoValidateBasic(t *testing.T) { srcMutator: func(c *ContractInfo) { c.Label = strings.Repeat("a", MaxLabelSize+1) }, expError: true, }, - "created nil": { - srcMutator: func(c *ContractInfo) { c.Created = nil }, - expError: true, - }, - "created invalid": { - srcMutator: func(c *ContractInfo) { c.Created = &AbsoluteTxPosition{BlockHeight: -1} }, - expError: true, - }, } for msg, spec := range specs { t.Run(msg, func(t *testing.T) { From 530710e67f244a0ac7a192a6ac4b9c0d0f846444 Mon Sep 17 00:00:00 2001 From: Alex Peters Date: Fri, 17 Jul 2020 09:49:43 +0200 Subject: [PATCH 3/4] Validate InstantiateConfig in CodeInfo --- x/wasm/internal/keeper/genesis_test.go | 105 ++++++++----------------- x/wasm/internal/types/test_fixtures.go | 18 ++--- x/wasm/internal/types/types.go | 3 + x/wasm/internal/types/types_test.go | 4 + 4 files changed, 45 insertions(+), 85 deletions(-) diff --git a/x/wasm/internal/keeper/genesis_test.go b/x/wasm/internal/keeper/genesis_test.go index d4544bb773..cefbc46df8 100644 --- a/x/wasm/internal/keeper/genesis_test.go +++ b/x/wasm/internal/keeper/genesis_test.go @@ -104,9 +104,8 @@ func TestGenesisExportImport(t *testing.T) { func TestFailFastImport(t *testing.T) { wasmCode, err := ioutil.ReadFile("./testdata/contract.wasm") require.NoError(t, err) - codeHash := sha256.Sum256(wasmCode) - anyAddress := make([]byte, 20) + myCodeInfo := wasmTypes.CodeInfoFixture(wasmTypes.WithSHA256CodeHash(wasmCode)) specs := map[string]struct { src types.GenesisState expSuccess bool @@ -114,11 +113,8 @@ func TestFailFastImport(t *testing.T) { "happy path: code info correct": { src: types.GenesisState{ Codes: []types.Code{{ - CodeID: 1, - CodeInfo: wasmTypes.CodeInfo{ - CodeHash: codeHash[:], - Creator: anyAddress, - }, + CodeID: 1, + CodeInfo: myCodeInfo, CodesBytes: wasmCode, }}, Sequences: []types.Sequence{ @@ -132,18 +128,12 @@ func TestFailFastImport(t *testing.T) { "happy path: code ids can contain gaps": { src: types.GenesisState{ Codes: []types.Code{{ - CodeID: 1, - CodeInfo: wasmTypes.CodeInfo{ - CodeHash: codeHash[:], - Creator: anyAddress, - }, + CodeID: 1, + CodeInfo: myCodeInfo, CodesBytes: wasmCode, }, { - CodeID: 3, - CodeInfo: wasmTypes.CodeInfo{ - CodeHash: codeHash[:], - Creator: anyAddress, - }, + CodeID: 3, + CodeInfo: myCodeInfo, CodesBytes: wasmCode, }}, Sequences: []types.Sequence{ @@ -157,18 +147,12 @@ func TestFailFastImport(t *testing.T) { "happy path: code order does not matter": { src: types.GenesisState{ Codes: []types.Code{{ - CodeID: 2, - CodeInfo: wasmTypes.CodeInfo{ - CodeHash: codeHash[:], - Creator: anyAddress, - }, + CodeID: 2, + CodeInfo: myCodeInfo, CodesBytes: wasmCode, }, { - CodeID: 1, - CodeInfo: wasmTypes.CodeInfo{ - CodeHash: codeHash[:], - Creator: anyAddress, - }, + CodeID: 1, + CodeInfo: myCodeInfo, CodesBytes: wasmCode, }}, Contracts: nil, @@ -182,11 +166,8 @@ func TestFailFastImport(t *testing.T) { }, "prevent code hash mismatch": {src: types.GenesisState{ Codes: []types.Code{{ - CodeID: 1, - CodeInfo: wasmTypes.CodeInfo{ - CodeHash: make([]byte, len(codeHash)), - Creator: anyAddress, - }, + CodeID: 1, + CodeInfo: wasmTypes.CodeInfoFixture(func(i *wasmTypes.CodeInfo) { i.CodeHash = make([]byte, sha256.Size) }), CodesBytes: wasmCode, }}, Params: types.DefaultParams(), @@ -194,19 +175,13 @@ func TestFailFastImport(t *testing.T) { "prevent duplicate codeIDs": {src: types.GenesisState{ Codes: []types.Code{ { - CodeID: 1, - CodeInfo: wasmTypes.CodeInfo{ - CodeHash: codeHash[:], - Creator: anyAddress, - }, + CodeID: 1, + CodeInfo: myCodeInfo, CodesBytes: wasmCode, }, { - CodeID: 1, - CodeInfo: wasmTypes.CodeInfo{ - CodeHash: codeHash[:], - Creator: anyAddress, - }, + CodeID: 1, + CodeInfo: myCodeInfo, CodesBytes: wasmCode, }, }, @@ -215,11 +190,8 @@ func TestFailFastImport(t *testing.T) { "happy path: code id in info and contract do match": { src: types.GenesisState{ Codes: []types.Code{{ - CodeID: 1, - CodeInfo: wasmTypes.CodeInfo{ - CodeHash: codeHash[:], - Creator: anyAddress, - }, + CodeID: 1, + CodeInfo: myCodeInfo, CodesBytes: wasmCode, }}, Contracts: []types.Contract{ @@ -239,11 +211,8 @@ func TestFailFastImport(t *testing.T) { "happy path: code info with two contracts": { src: types.GenesisState{ Codes: []types.Code{{ - CodeID: 1, - CodeInfo: wasmTypes.CodeInfo{ - CodeHash: codeHash[:], - Creator: anyAddress, - }, + CodeID: 1, + CodeInfo: myCodeInfo, CodesBytes: wasmCode, }}, Contracts: []types.Contract{ @@ -277,11 +246,8 @@ func TestFailFastImport(t *testing.T) { "prevent duplicate contract address": { src: types.GenesisState{ Codes: []types.Code{{ - CodeID: 1, - CodeInfo: wasmTypes.CodeInfo{ - CodeHash: codeHash[:], - Creator: anyAddress, - }, + CodeID: 1, + CodeInfo: myCodeInfo, CodesBytes: wasmCode, }}, Contracts: []types.Contract{ @@ -299,11 +265,8 @@ func TestFailFastImport(t *testing.T) { "prevent duplicate contract model keys": { src: types.GenesisState{ Codes: []types.Code{{ - CodeID: 1, - CodeInfo: wasmTypes.CodeInfo{ - CodeHash: codeHash[:], - Creator: anyAddress, - }, + CodeID: 1, + CodeInfo: myCodeInfo, CodesBytes: wasmCode, }}, Contracts: []types.Contract{ @@ -337,11 +300,8 @@ func TestFailFastImport(t *testing.T) { "prevent code id seq init value == max codeID used": { src: types.GenesisState{ Codes: []types.Code{{ - CodeID: 2, - CodeInfo: wasmTypes.CodeInfo{ - CodeHash: codeHash[:], - Creator: anyAddress, - }, + CodeID: 2, + CodeInfo: myCodeInfo, CodesBytes: wasmCode, }}, Sequences: []types.Sequence{ @@ -353,11 +313,8 @@ func TestFailFastImport(t *testing.T) { "prevent contract id seq init value == count contracts": { src: types.GenesisState{ Codes: []types.Code{{ - CodeID: 1, - CodeInfo: wasmTypes.CodeInfo{ - CodeHash: codeHash[:], - Creator: anyAddress, - }, + CodeID: 1, + CodeInfo: myCodeInfo, CodesBytes: wasmCode, }}, Contracts: []types.Contract{ @@ -456,8 +413,8 @@ func TestImportContractWithCodeHistoryReset(t *testing.T) { "source": "https://example.com", "builder": "foo/bar:tag", "instantiate_config": { - "type": 1, - "address": "" + "permission": "OnlyAddress", + "address": "cosmos1qtu5n0cnhfkjj6l2rq97hmky9fd89gwca9yarx" } }, "code_bytes": %q diff --git a/x/wasm/internal/types/test_fixtures.go b/x/wasm/internal/types/test_fixtures.go index 22dbfe12c2..2bcbf6319a 100644 --- a/x/wasm/internal/types/test_fixtures.go +++ b/x/wasm/internal/types/test_fixtures.go @@ -42,15 +42,10 @@ func GenesisFixture(mutators ...func(*GenesisState)) GenesisState { func CodeFixture(mutators ...func(*Code)) Code { wasmCode := rand.Bytes(100) - codeHash := sha256.Sum256(wasmCode) - anyAddress := make([]byte, 20) fixture := Code{ - CodeID: 1, - CodeInfo: CodeInfo{ - CodeHash: codeHash[:], - Creator: anyAddress, - }, + CodeID: 1, + CodeInfo: CodeInfoFixture(WithSHA256CodeHash(wasmCode)), CodesBytes: wasmCode, } @@ -65,10 +60,11 @@ func CodeInfoFixture(mutators ...func(*CodeInfo)) CodeInfo { codeHash := sha256.Sum256(wasmCode) anyAddress := make([]byte, 20) fixture := CodeInfo{ - CodeHash: codeHash[:], - Creator: anyAddress, - Source: "https://example.com", - Builder: "my/builder:tag", + CodeHash: codeHash[:], + Creator: anyAddress, + Source: "https://example.com", + Builder: "my/builder:tag", + InstantiateConfig: AllowEverybody, } for _, m := range mutators { m(&fixture) diff --git a/x/wasm/internal/types/types.go b/x/wasm/internal/types/types.go index c6d69f8286..92ff0df180 100644 --- a/x/wasm/internal/types/types.go +++ b/x/wasm/internal/types/types.go @@ -51,6 +51,9 @@ func (c CodeInfo) ValidateBasic() error { if err := validateBuilder(c.Builder); err != nil { return sdkerrors.Wrap(err, "builder") } + if err := c.InstantiateConfig.ValidateBasic(); err != nil { + return sdkerrors.Wrap(err, "instantiate config") + } return nil } diff --git a/x/wasm/internal/types/types_test.go b/x/wasm/internal/types/types_test.go index ad63c476af..e2e6d31cf6 100644 --- a/x/wasm/internal/types/types_test.go +++ b/x/wasm/internal/types/types_test.go @@ -101,6 +101,10 @@ func TestCodeInfoValidateBasic(t *testing.T) { srcMutator: func(c *CodeInfo) { c.Builder = "invalid" }, expError: true, }, + "Instantiate config invalid": { + srcMutator: func(c *CodeInfo) { c.InstantiateConfig = AccessConfig{} }, + expError: true, + }, } for msg, spec := range specs { t.Run(msg, func(t *testing.T) { From a9f31c7cd83bd1bb4bd5b6c27f3d1db17a566676 Mon Sep 17 00:00:00 2001 From: Alex Peters Date: Fri, 17 Jul 2020 10:40:32 +0200 Subject: [PATCH 4/4] Additional tests and minor updates --- x/wasm/internal/keeper/keeper.go | 1 + x/wasm/internal/types/msg.go | 4 +- x/wasm/internal/types/msg_test.go | 92 ++++++++++++++++++++++++++++ x/wasm/internal/types/params.go | 2 +- x/wasm/internal/types/params_test.go | 16 ++++- x/wasm/internal/types/types.go | 10 --- 6 files changed, 110 insertions(+), 15 deletions(-) diff --git a/x/wasm/internal/keeper/keeper.go b/x/wasm/internal/keeper/keeper.go index d781a28d11..f8c3ec9a17 100644 --- a/x/wasm/internal/keeper/keeper.go +++ b/x/wasm/internal/keeper/keeper.go @@ -169,6 +169,7 @@ func (k Keeper) importCode(ctx sdk.Context, codeID uint64, codeInfo types.CodeIn func (k Keeper) Instantiate(ctx sdk.Context, codeID uint64, creator, admin sdk.AccAddress, initMsg []byte, label string, deposit sdk.Coins) (sdk.AccAddress, error) { return k.instantiate(ctx, codeID, creator, admin, initMsg, label, deposit, k.authZPolicy) } + func (k Keeper) instantiate(ctx sdk.Context, codeID uint64, creator, admin sdk.AccAddress, initMsg []byte, label string, deposit sdk.Coins, authZ AuthorizationPolicy) (sdk.AccAddress, error) { ctx.GasMeter().ConsumeGas(InstanceCost, "Loading CosmWasm module: init") diff --git a/x/wasm/internal/types/msg.go b/x/wasm/internal/types/msg.go index 88c7a2dc29..c496c362f7 100644 --- a/x/wasm/internal/types/msg.go +++ b/x/wasm/internal/types/msg.go @@ -133,8 +133,8 @@ func (msg MsgExecuteContract) ValidateBasic() error { return err } - if msg.SentFunds.IsAnyNegative() { - return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, "negative SentFunds") + if !msg.SentFunds.IsValid() { + return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, "sentFunds") } return nil } diff --git a/x/wasm/internal/types/msg_test.go b/x/wasm/internal/types/msg_test.go index 588cf565a7..858658386e 100644 --- a/x/wasm/internal/types/msg_test.go +++ b/x/wasm/internal/types/msg_test.go @@ -218,6 +218,98 @@ func TestInstantiateContractValidation(t *testing.T) { } } +func TestExecuteContractValidation(t *testing.T) { + badAddress, err := sdk.AccAddressFromHex("012345") + require.NoError(t, err) + // proper address size + goodAddress := sdk.AccAddress(make([]byte, 20)) + + cases := map[string]struct { + msg MsgExecuteContract + valid bool + }{ + "empty": { + msg: MsgExecuteContract{}, + valid: false, + }, + "correct minimal": { + msg: MsgExecuteContract{ + Sender: goodAddress, + Contract: goodAddress, + }, + valid: true, + }, + "correct all": { + msg: MsgExecuteContract{ + Sender: goodAddress, + Contract: goodAddress, + Msg: []byte(`{"some": "data"}`), + SentFunds: sdk.Coins{sdk.Coin{Denom: "foobar", Amount: sdk.NewInt(200)}}, + }, + valid: true, + }, + "bad sender": { + msg: MsgExecuteContract{ + Sender: badAddress, + Contract: goodAddress, + Msg: []byte(`{"some": "data"}`), + }, + valid: false, + }, + "empty sender": { + msg: MsgExecuteContract{ + Contract: goodAddress, + Msg: []byte(`{"some": "data"}`), + }, + valid: false, + }, + "bad contract": { + msg: MsgExecuteContract{ + Sender: goodAddress, + Contract: badAddress, + Msg: []byte(`{"some": "data"}`), + }, + valid: false, + }, + "empty contract": { + msg: MsgExecuteContract{ + Sender: goodAddress, + Msg: []byte(`{"some": "data"}`), + }, + valid: false, + }, + "negative funds": { + msg: MsgExecuteContract{ + Sender: goodAddress, + Contract: goodAddress, + Msg: []byte(`{"some": "data"}`), + SentFunds: sdk.Coins{sdk.Coin{Denom: "foobar", Amount: sdk.NewInt(-1)}}, + }, + valid: false, + }, + "duplicate funds": { + msg: MsgExecuteContract{ + Sender: goodAddress, + Contract: goodAddress, + Msg: []byte(`{"some": "data"}`), + SentFunds: sdk.Coins{sdk.Coin{Denom: "foobar", Amount: sdk.NewInt(1)}, sdk.Coin{Denom: "foobar", Amount: sdk.NewInt(1)}}, + }, + valid: false, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + err := tc.msg.ValidateBasic() + if tc.valid { + assert.NoError(t, err) + } else { + assert.Error(t, err) + } + }) + } +} + func TestMsgUpdateAdministrator(t *testing.T) { badAddress, err := sdk.AccAddressFromHex("012345") require.NoError(t, err) diff --git a/x/wasm/internal/types/params.go b/x/wasm/internal/types/params.go index 54a89fe703..86de2a1e0e 100644 --- a/x/wasm/internal/types/params.go +++ b/x/wasm/internal/types/params.go @@ -147,7 +147,7 @@ func validateAccessType(i interface{}) error { func (v AccessConfig) ValidateBasic() error { switch v.Type { - case Undefined: + case Undefined, "": return sdkerrors.Wrap(ErrEmpty, "type") case Nobody, Everybody: if len(v.Address) != 0 { diff --git a/x/wasm/internal/types/params_test.go b/x/wasm/internal/types/params_test.go index 8bbecc61ba..0c78b148da 100644 --- a/x/wasm/internal/types/params_test.go +++ b/x/wasm/internal/types/params_test.go @@ -61,20 +61,32 @@ func TestValidateParams(t *testing.T) { }, expErr: true, }, - "reject AccessConfig Everybody with obsolete address": { + "reject UploadAccess Everybody with obsolete address": { src: Params{ UploadAccess: AccessConfig{Type: Everybody, Address: anyAddress}, DefaultInstantiatePermission: OnlyAddress, }, expErr: true, }, - "reject AccessConfig Nobody with obsolete address": { + "reject UploadAccess Nobody with obsolete address": { src: Params{ UploadAccess: AccessConfig{Type: Nobody, Address: anyAddress}, DefaultInstantiatePermission: OnlyAddress, }, expErr: true, }, + "reject empty UploadAccess": { + src: Params{ + DefaultInstantiatePermission: OnlyAddress, + }, + expErr: true, + }, "reject undefined permission in UploadAccess": { + src: Params{ + UploadAccess: AccessConfig{Type: Undefined}, + DefaultInstantiatePermission: OnlyAddress, + }, + expErr: true, + }, } for msg, spec := range specs { t.Run(msg, func(t *testing.T) { diff --git a/x/wasm/internal/types/types.go b/x/wasm/internal/types/types.go index 92ff0df180..7725c9f0e6 100644 --- a/x/wasm/internal/types/types.go +++ b/x/wasm/internal/types/types.go @@ -159,16 +159,6 @@ func (a *AbsoluteTxPosition) LessThan(b *AbsoluteTxPosition) bool { return a.BlockHeight < b.BlockHeight || (a.BlockHeight == b.BlockHeight && a.TxIndex < b.TxIndex) } -func (a *AbsoluteTxPosition) ValidateBasic() error { - if a == nil { - return nil - } - if a.BlockHeight < 0 { - return sdkerrors.Wrap(ErrInvalid, "height") - } - return nil -} - // NewAbsoluteTxPosition gets a timestamp from the context func NewAbsoluteTxPosition(ctx sdk.Context) *AbsoluteTxPosition { // we must safely handle nil gas meters