From a20e568bffbae06016af3f77616de5bfec153e62 Mon Sep 17 00:00:00 2001 From: Alex Peters Date: Tue, 30 Jun 2020 11:08:45 +0200 Subject: [PATCH 1/3] Validate genesis model --- x/wasm/internal/keeper/genesis.go | 5 +- x/wasm/internal/keeper/genesis_test.go | 163 ++++++++++++++++++++++++- x/wasm/internal/keeper/keeper.go | 32 ++++- x/wasm/internal/keeper/querier_test.go | 2 +- x/wasm/internal/types/errors.go | 9 ++ x/wasm/internal/types/genesis.go | 58 ++++++++- x/wasm/internal/types/msg.go | 72 ++--------- x/wasm/internal/types/types.go | 42 +++++++ x/wasm/internal/types/validation.go | 80 ++++++++++++ 9 files changed, 392 insertions(+), 71 deletions(-) create mode 100644 x/wasm/internal/types/validation.go diff --git a/x/wasm/internal/keeper/genesis.go b/x/wasm/internal/keeper/genesis.go index 96e39022b8..b23ec7cec6 100644 --- a/x/wasm/internal/keeper/genesis.go +++ b/x/wasm/internal/keeper/genesis.go @@ -25,12 +25,11 @@ func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) { } for _, contract := range data.Contracts { - keeper.setContractInfo(ctx, contract.ContractAddress, &contract.ContractInfo) - keeper.setContractState(ctx, contract.ContractAddress, contract.ContractState) + keeper.importContract(ctx, contract.ContractAddress, &contract.ContractInfo, contract.ContractState) } for _, seq := range data.Sequences { - keeper.setAutoIncrementID(ctx, seq.IDKey, seq.Value) + keeper.importAutoIncrementID(ctx, seq.IDKey, seq.Value) } } diff --git a/x/wasm/internal/keeper/genesis_test.go b/x/wasm/internal/keeper/genesis_test.go index 88963c4af7..748135142a 100644 --- a/x/wasm/internal/keeper/genesis_test.go +++ b/x/wasm/internal/keeper/genesis_test.go @@ -1,6 +1,7 @@ package keeper import ( + "crypto/sha256" "io/ioutil" "os" "testing" @@ -42,7 +43,7 @@ func TestGenesisExportImport(t *testing.T) { contract.CodeID = codeID contractAddr := srcKeeper.generateContractAddress(srcCtx, codeID) srcKeeper.setContractInfo(srcCtx, contractAddr, &contract) - srcKeeper.setContractState(srcCtx, contractAddr, stateModels) + srcKeeper.importContractState(srcCtx, contractAddr, stateModels) } // export @@ -67,6 +68,166 @@ func TestGenesisExportImport(t *testing.T) { require.False(t, dstIT.Valid()) } +func TestFailFastImport(t *testing.T) { + wasmCode, err := ioutil.ReadFile("./testdata/contract.wasm") + require.NoError(t, err) + codeHash := sha256.Sum256(wasmCode) + anyAddress := make([]byte, 20) + + specs := map[string]struct { + src types.GenesisState + expSuccess bool + }{ + "happy path: code info correct": { + src: types.GenesisState{ + Codes: []types.Code{{ + CodeInfo: wasmTypes.CodeInfo{ + CodeHash: codeHash[:], + Creator: anyAddress, + }, + CodesBytes: wasmCode, + }}, + Contracts: nil, + }, + expSuccess: true, + }, + "prevent code hash mismatch": {src: types.GenesisState{ + Codes: []types.Code{{ + CodeInfo: wasmTypes.CodeInfo{ + CodeHash: make([]byte, len(codeHash)), + Creator: anyAddress, + }, + CodesBytes: wasmCode, + }}, + Contracts: nil, + }}, + "happy path: code info and contract do match": { + src: types.GenesisState{ + Codes: []types.Code{{ + CodeInfo: wasmTypes.CodeInfo{ + CodeHash: codeHash[:], + Creator: anyAddress, + }, + CodesBytes: wasmCode, + }}, + Contracts: []types.Contract{ + { + ContractAddress: addrFromUint64(1<<32 + 1), + ContractInfo: wasmTypes.ContractInfo{ + CodeID: 1, + Creator: anyAddress, + Label: "any", + Created: &types.AbsoluteTxPosition{BlockHeight: 1, TxIndex: 1}, + }, + }, + }, + }, + expSuccess: true, + }, + "prevent contracts that points to non existing codeID": { + src: types.GenesisState{ + Contracts: []types.Contract{ + { + ContractAddress: addrFromUint64(1<<32 + 1), + ContractInfo: wasmTypes.ContractInfo{ + CodeID: 1, + Creator: anyAddress, + Label: "any", + Created: &types.AbsoluteTxPosition{BlockHeight: 1, TxIndex: 1}, + }, + }, + }, + }, + }, + "prevent duplicate contracts": { + src: types.GenesisState{ + Codes: []types.Code{{ + CodeInfo: wasmTypes.CodeInfo{ + CodeHash: codeHash[:], + Creator: anyAddress, + }, + CodesBytes: wasmCode, + }}, + Contracts: []types.Contract{ + { + ContractAddress: addrFromUint64(1<<32 + 1), + ContractInfo: wasmTypes.ContractInfo{ + CodeID: 1, + Creator: anyAddress, + Label: "any", + Created: &types.AbsoluteTxPosition{BlockHeight: 1, TxIndex: 1}, + }, + }, { + ContractAddress: addrFromUint64(1<<32 + 1), + ContractInfo: wasmTypes.ContractInfo{ + CodeID: 1, + Creator: anyAddress, + Label: "any", + Created: &types.AbsoluteTxPosition{BlockHeight: 1, TxIndex: 1}, + }, + }, + }, + }, + }, + "prevent duplicate contract model": { + src: types.GenesisState{ + Codes: []types.Code{{ + CodeInfo: wasmTypes.CodeInfo{ + CodeHash: codeHash[:], + Creator: anyAddress, + }, + CodesBytes: wasmCode, + }}, + Contracts: []types.Contract{ + { + ContractAddress: addrFromUint64(1<<32 + 1), + ContractInfo: wasmTypes.ContractInfo{ + CodeID: 1, + Creator: anyAddress, + Label: "any", + Created: &types.AbsoluteTxPosition{BlockHeight: 1, TxIndex: 1}, + }, + ContractState: []types.Model{ + { + Key: []byte{0x1}, + Value: []byte("foo"), + }, + { + Key: []byte{0x1}, + Value: []byte("bar"), + }, + }, + }, + }, + }, + }, + "prevent duplicate sequences": { + src: types.GenesisState{ + Sequences: []types.Sequence{ + {IDKey: []byte("foo"), Value: 1}, + {IDKey: []byte("foo"), Value: 9999}, + }, + }, + }, + } + + for msg, spec := range specs { + t.Run(msg, func(t *testing.T) { + keeper, ctx, cleanup := setupKeeper(t) + defer cleanup() + + require.NoError(t, types.ValidateGenesis(spec.src)) + if spec.expSuccess { + InitGenesis(ctx, keeper, spec.src) + return + } + require.Panics(t, func() { + InitGenesis(ctx, keeper, spec.src) + }) + }) + } +} + func setupKeeper(t *testing.T) (Keeper, sdk.Context, func()) { tempDir, err := ioutil.TempDir("", "wasm") require.NoError(t, err) diff --git a/x/wasm/internal/keeper/keeper.go b/x/wasm/internal/keeper/keeper.go index b2b9756043..74aa88eac6 100644 --- a/x/wasm/internal/keeper/keeper.go +++ b/x/wasm/internal/keeper/keeper.go @@ -2,6 +2,7 @@ package keeper import ( "encoding/binary" + "fmt" "path/filepath" "github.com/cosmos/cosmos-sdk/x/staking" @@ -374,6 +375,11 @@ func (k Keeper) GetContractInfo(ctx sdk.Context, contractAddress sdk.AccAddress) return &contract } +func (k Keeper) containsContractInfo(ctx sdk.Context, contractAddress sdk.AccAddress) bool { + store := ctx.KVStore(k.storeKey) + return store.Has(types.GetContractAddressKey(contractAddress)) +} + func (k Keeper) setContractInfo(ctx sdk.Context, contractAddress sdk.AccAddress, contract *types.ContractInfo) { store := ctx.KVStore(k.storeKey) store.Set(types.GetContractAddressKey(contractAddress), k.cdc.MustMarshalBinaryBare(contract)) @@ -398,13 +404,16 @@ func (k Keeper) GetContractState(ctx sdk.Context, contractAddress sdk.AccAddress return prefixStore.Iterator(nil, nil) } -func (k Keeper) setContractState(ctx sdk.Context, contractAddress sdk.AccAddress, models []types.Model) { +func (k Keeper) importContractState(ctx sdk.Context, contractAddress sdk.AccAddress, models []types.Model) { prefixStoreKey := types.GetContractStorePrefixKey(contractAddress) prefixStore := prefix.NewStore(ctx.KVStore(k.storeKey), prefixStoreKey) for _, model := range models { if model.Value == nil { model.Value = []byte{} } + if prefixStore.Has(model.Key) { + panic(fmt.Sprintf("duplicate key: %x", model.Key)) + } prefixStore.Set(model.Key, model.Value) } } @@ -420,6 +429,11 @@ func (k Keeper) GetCodeInfo(ctx sdk.Context, codeID uint64) *types.CodeInfo { return &codeInfo } +func (k Keeper) containsCodeInfo(ctx sdk.Context, codeID uint64) bool { + store := ctx.KVStore(k.storeKey) + return store.Has(types.GetCodeKey(codeID)) +} + func (k Keeper) GetByteCode(ctx sdk.Context, codeID uint64) ([]byte, error) { store := ctx.KVStore(k.storeKey) var codeInfo types.CodeInfo @@ -496,12 +510,26 @@ func (k Keeper) peekAutoIncrementID(ctx sdk.Context, lastIDKey []byte) uint64 { return id } -func (k Keeper) setAutoIncrementID(ctx sdk.Context, lastIDKey []byte, val uint64) { +func (k Keeper) importAutoIncrementID(ctx sdk.Context, lastIDKey []byte, val uint64) { store := ctx.KVStore(k.storeKey) + if store.Has(lastIDKey) { + panic(fmt.Sprintf("duplicate autoincrement id: %s", string(lastIDKey))) + } bz := sdk.Uint64ToBigEndian(val) store.Set(lastIDKey, bz) } +func (k Keeper) importContract(ctx sdk.Context, address sdk.AccAddress, c *types.ContractInfo, state []types.Model) { + if !k.containsCodeInfo(ctx, c.CodeID) { + panic(fmt.Sprintf("unknown code id: %d", c.CodeID)) + } + if k.containsContractInfo(ctx, address) { + panic(fmt.Sprintf("duplicate contract: %s", address)) + } + k.setContractInfo(ctx, address, c) + k.importContractState(ctx, address, state) +} + func addrFromUint64(id uint64) sdk.AccAddress { addr := make([]byte, 20) addr[0] = 'C' diff --git a/x/wasm/internal/keeper/querier_test.go b/x/wasm/internal/keeper/querier_test.go index aa6d34fd7d..13f28054a4 100644 --- a/x/wasm/internal/keeper/querier_test.go +++ b/x/wasm/internal/keeper/querier_test.go @@ -48,7 +48,7 @@ func TestQueryContractState(t *testing.T) { {Key: []byte("foo"), Value: []byte(`"bar"`)}, {Key: []byte{0x0, 0x1}, Value: []byte(`{"count":8}`)}, } - keeper.setContractState(ctx, addr, contractModel) + keeper.importContractState(ctx, addr, contractModel) // this gets us full error, not redacted sdk.Error q := NewQuerier(keeper) diff --git a/x/wasm/internal/types/errors.go b/x/wasm/internal/types/errors.go index f54d272979..3ab5b5c450 100644 --- a/x/wasm/internal/types/errors.go +++ b/x/wasm/internal/types/errors.go @@ -37,4 +37,13 @@ var ( // ErrMigrationFailed error for rust execution contract failure ErrMigrationFailed = sdkErrors.Register(DefaultCodespace, 10, "migrate wasm contract failed") + + // ErrEmpty error for empty content + ErrEmpty = sdkErrors.Register(DefaultCodespace, 11, "empty") + + // ErrLimit error for content that exceeds a limit + ErrLimit = sdkErrors.Register(DefaultCodespace, 12, "exceeds limit") + + // ErrInvalid error for content that is invalid in this context + ErrInvalid = sdkErrors.Register(DefaultCodespace, 13, "invalid") ) diff --git a/x/wasm/internal/types/genesis.go b/x/wasm/internal/types/genesis.go index 1a36fff6ec..e71a987b56 100644 --- a/x/wasm/internal/types/genesis.go +++ b/x/wasm/internal/types/genesis.go @@ -1,12 +1,22 @@ package types -import sdk "github.com/cosmos/cosmos-sdk/types" +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" +) type Sequence struct { IDKey []byte `json:"id_key"` Value uint64 `json:"value"` } +func (s Sequence) ValidateBasic() error { + if len(s.IDKey) == 0 { + return sdkerrors.Wrap(ErrEmpty, "id key") + } + return nil +} + // GenesisState is the struct representation of the export genesis type GenesisState struct { Codes []Code `json:"codes"` @@ -14,12 +24,41 @@ type GenesisState struct { Sequences []Sequence `json:"sequences"` } +func (s GenesisState) ValidateBasic() error { + for i := range s.Codes { + if err := s.Codes[i].ValidateBasic(); err != nil { + return sdkerrors.Wrapf(err, "code: %d", i) + } + } + for i := range s.Contracts { + if err := s.Contracts[i].ValidateBasic(); err != nil { + return sdkerrors.Wrapf(err, "contract: %d", i) + } + } + for i := range s.Sequences { + if err := s.Sequences[i].ValidateBasic(); err != nil { + return sdkerrors.Wrapf(err, "sequence: %d", i) + } + } + return nil +} + // Code struct encompasses CodeInfo and CodeBytes type Code struct { CodeInfo CodeInfo `json:"code_info"` CodesBytes []byte `json:"code_bytes"` } +func (c Code) ValidateBasic() error { + if err := c.CodeInfo.ValidateBasic(); err != nil { + return sdkerrors.Wrap(err, "code info") + } + if err := validateWasmCode(c.CodesBytes); err != nil { + return sdkerrors.Wrap(err, "code bytes") + } + return nil +} + // Contract struct encompasses ContractAddress, ContractInfo, and ContractState type Contract struct { ContractAddress sdk.AccAddress `json:"contract_address"` @@ -27,8 +66,23 @@ type Contract struct { ContractState []Model `json:"contract_state"` } +func (c Contract) ValidateBasic() error { + if err := sdk.VerifyAddressFormat(c.ContractAddress); err != nil { + return sdkerrors.Wrap(err, "contract address") + } + if err := c.ContractInfo.ValidateBasic(); err != nil { + return sdkerrors.Wrap(err, "contract info") + } + for i := range c.ContractState { + if err := c.ContractState[i].ValidateBasic(); err != nil { + return sdkerrors.Wrapf(err, "contract state %d", i) + } + } + return nil +} + // ValidateGenesis performs basic validation of supply genesis data returning an // error for any failed validation criteria. func ValidateGenesis(data GenesisState) error { - return nil + return data.ValidateBasic() } diff --git a/x/wasm/internal/types/msg.go b/x/wasm/internal/types/msg.go index 6955986607..f638c286ac 100644 --- a/x/wasm/internal/types/msg.go +++ b/x/wasm/internal/types/msg.go @@ -2,35 +2,11 @@ package types import ( "encoding/json" - "net/url" - "regexp" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) -const ( - MaxWasmSize = 500 * 1024 - - // MaxLabelSize is the longest label that can be used when Instantiating a contract - MaxLabelSize = 128 - - // BuildTagRegexp is a docker image regexp. - // We only support max 128 characters, with at least one organization name (subset of all legal names). - // - // Details from https://docs.docker.com/engine/reference/commandline/tag/#extended-description : - // - // An image name is made up of slash-separated name components (optionally prefixed by a registry hostname). - // Name components may contain lowercase characters, digits and separators. - // A separator is defined as a period, one or two underscores, or one or more dashes. A name component may not start or end with a separator. - // - // A tag name must be valid ASCII and may contain lowercase and uppercase letters, digits, underscores, periods and dashes. - // A tag name may not start with a period or a dash and may contain a maximum of 128 characters. - BuildTagRegexp = "^[a-z0-9][a-z0-9._-]*[a-z0-9](/[a-z0-9][a-z0-9._-]*[a-z0-9])+:[a-zA-Z0-9_][a-zA-Z0-9_.-]*$" - - MaxBuildTagSize = 128 -) - type MsgStoreCode struct { Sender sdk.AccAddress `json:"sender" yaml:"sender"` // WASMByteCode can be raw or gzip compressed @@ -54,28 +30,18 @@ func (msg MsgStoreCode) ValidateBasic() error { return err } - if len(msg.WASMByteCode) == 0 { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "empty wasm code") + if err := validateWasmCode(msg.WASMByteCode); err != nil { + return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "code bytes %s", err.Error()) } - if len(msg.WASMByteCode) > MaxWasmSize { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "wasm code too large") + if err := validateSourceURL(msg.Source); err != nil { + return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "source %s", err.Error()) } - if msg.Source != "" { - u, err := url.Parse(msg.Source) - if err != nil { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "source should be a valid url") - } - if !u.IsAbs() { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "source should be an absolute url") - } - if u.Scheme != "https" { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "source must use https") - } + if err := validateBuilder(msg.Builder); err != nil { + return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "builder %s", err.Error()) } - - return validateBuilder(msg.Builder) + return nil } func (msg MsgStoreCode) GetSignBytes() []byte { @@ -86,21 +52,6 @@ func (msg MsgStoreCode) GetSigners() []sdk.AccAddress { return []sdk.AccAddress{msg.Sender} } -func validateBuilder(buildTag string) error { - if len(buildTag) > MaxBuildTagSize { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "builder tag longer than 128 characters") - } - - if buildTag != "" { - ok, err := regexp.MatchString(BuildTagRegexp, buildTag) - if err != nil || !ok { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "invalid tag supplied for builder") - } - } - - return nil -} - type MsgInstantiateContract struct { Sender sdk.AccAddress `json:"sender" yaml:"sender"` // Admin is an optional address that can execute migrations @@ -127,11 +78,9 @@ func (msg MsgInstantiateContract) ValidateBasic() error { if msg.Code == 0 { return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "code_id is required") } - if msg.Label == "" { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "label is required") - } - if len(msg.Label) > MaxLabelSize { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "label cannot be longer than 128 characters") + + if err := validateLabel(msg.Label); err != nil { + return err } if msg.InitFunds.IsAnyNegative() { @@ -143,7 +92,6 @@ func (msg MsgInstantiateContract) ValidateBasic() error { return err } } - return nil } diff --git a/x/wasm/internal/types/types.go b/x/wasm/internal/types/types.go index 729a6a47d9..1f6cbe10ef 100644 --- a/x/wasm/internal/types/types.go +++ b/x/wasm/internal/types/types.go @@ -3,6 +3,7 @@ package types import ( "encoding/json" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" tmBytes "github.com/tendermint/tendermint/libs/bytes" wasmTypes "github.com/CosmWasm/go-cosmwasm/types" @@ -20,6 +21,13 @@ type Model struct { Value []byte `json:"val"` } +func (m Model) ValidateBasic() error { + if len(m.Key) == 0 { + return sdkerrors.Wrap(ErrEmpty, "key") + } + return nil +} + // CodeInfo is data for the uploaded contract WASM code type CodeInfo struct { CodeHash []byte `json:"code_hash"` @@ -28,6 +36,22 @@ type CodeInfo struct { Builder string `json:"builder"` } +func (c CodeInfo) ValidateBasic() error { + if len(c.CodeHash) == 0 { + return sdkerrors.Wrap(ErrEmpty, "code hash") + } + if err := sdk.VerifyAddressFormat(c.Creator); err != nil { + return sdkerrors.Wrap(err, "creator") + } + if err := validateSourceURL(c.Source); err != nil { + return sdkerrors.Wrap(err, "source") + } + if err := validateBuilder(c.Builder); err != nil { + return sdkerrors.Wrap(err, "builder") + } + return nil +} + // NewCodeInfo fills a new Contract struct func NewCodeInfo(codeHash []byte, creator sdk.AccAddress, source string, builder string) CodeInfo { return CodeInfo{ @@ -58,6 +82,24 @@ func (c *ContractInfo) UpdateCodeID(ctx sdk.Context, newCodeID uint64) { c.LastUpdated = NewCreatedAt(ctx) } +func (c *ContractInfo) ValidateBasic() error { + if c.CodeID == 0 { + return sdkerrors.Wrap(ErrEmpty, "code id") + } + if err := sdk.VerifyAddressFormat(c.Creator); err != nil { + return sdkerrors.Wrap(err, "creator") + } + if c.Admin != nil { + if err := sdk.VerifyAddressFormat(c.Admin); err != nil { + return sdkerrors.Wrap(err, "admin") + } + } + if err := validateLabel(c.Label); err != nil { + return sdkerrors.Wrap(err, "label") + } + return nil +} + // 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/validation.go b/x/wasm/internal/types/validation.go new file mode 100644 index 0000000000..23c59661b1 --- /dev/null +++ b/x/wasm/internal/types/validation.go @@ -0,0 +1,80 @@ +package types + +import ( + "net/url" + "regexp" + + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" +) + +const ( + MaxWasmSize = 500 * 1024 + + // MaxLabelSize is the longest label that can be used when Instantiating a contract + MaxLabelSize = 128 + + // BuildTagRegexp is a docker image regexp. + // We only support max 128 characters, with at least one organization name (subset of all legal names). + // + // Details from https://docs.docker.com/engine/reference/commandline/tag/#extended-description : + // + // An image name is made up of slash-separated name components (optionally prefixed by a registry hostname). + // Name components may contain lowercase characters, digits and separators. + // A separator is defined as a period, one or two underscores, or one or more dashes. A name component may not start or end with a separator. + // + // A tag name must be valid ASCII and may contain lowercase and uppercase letters, digits, underscores, periods and dashes. + // A tag name may not start with a period or a dash and may contain a maximum of 128 characters. + BuildTagRegexp = "^[a-z0-9][a-z0-9._-]*[a-z0-9](/[a-z0-9][a-z0-9._-]*[a-z0-9])+:[a-zA-Z0-9_][a-zA-Z0-9_.-]*$" + + MaxBuildTagSize = 128 +) + +func validateSourceURL(source string) error { + if source != "" { + u, err := url.Parse(source) + if err != nil { + return sdkerrors.Wrap(ErrInvalid, "not an url") + } + if !u.IsAbs() { + return sdkerrors.Wrap(ErrInvalid, "not an absolute url") + } + if u.Scheme != "https" { + return sdkerrors.Wrap(ErrInvalid, "must use https") + } + } + return nil +} + +func validateBuilder(buildTag string) error { + if len(buildTag) > MaxBuildTagSize { + return sdkerrors.Wrap(ErrLimit, "longer than 128 characters") + } + + if buildTag != "" { + ok, err := regexp.MatchString(BuildTagRegexp, buildTag) + if err != nil || !ok { + return ErrInvalid + } + } + return nil +} + +func validateWasmCode(s []byte) error { + if len(s) == 0 { + return sdkerrors.Wrap(ErrEmpty, "is required") + } + if len(s) > MaxWasmSize { + return sdkerrors.Wrapf(ErrLimit, "cannot be longer than %d bytes", MaxWasmSize) + } + return nil +} + +func validateLabel(label string) error { + if label == "" { + return sdkerrors.Wrap(ErrEmpty, "is required") + } + if len(label) > MaxLabelSize { + return sdkerrors.Wrap(ErrLimit, "cannot be longer than 128 characters") + } + return nil +} From f7b4acf47cf20dfa92b6fa578d62a997f395ba36 Mon Sep 17 00:00:00 2001 From: Alex Peters Date: Tue, 30 Jun 2020 20:37:21 +0200 Subject: [PATCH 2/3] Review comments and additinal tests --- x/wasm/internal/keeper/genesis.go | 25 ++++-- x/wasm/internal/keeper/genesis_test.go | 44 ++++++++-- x/wasm/internal/keeper/keeper.go | 25 +++--- x/wasm/internal/types/errors.go | 3 + x/wasm/internal/types/genesis_test.go | 107 +++++++++++++++++++++++++ x/wasm/internal/types/types.go | 16 ++++ x/wasm/module.go | 4 +- 7 files changed, 199 insertions(+), 25 deletions(-) create mode 100644 x/wasm/internal/types/genesis_test.go diff --git a/x/wasm/internal/keeper/genesis.go b/x/wasm/internal/keeper/genesis.go index b23ec7cec6..04a4220e93 100644 --- a/x/wasm/internal/keeper/genesis.go +++ b/x/wasm/internal/keeper/genesis.go @@ -5,6 +5,7 @@ import ( "github.com/CosmWasm/wasmd/x/wasm/internal/types" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" // authexported "github.com/cosmos/cosmos-sdk/x/auth/exported" // "github.com/CosmWasm/wasmd/x/wasm/internal/types" ) @@ -12,25 +13,33 @@ import ( // InitGenesis sets supply information for genesis. // // CONTRACT: all types of accounts must have been already initialized/created -func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) { - for _, code := range data.Codes { +func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) error { + for i, code := range data.Codes { newId, err := keeper.Create(ctx, code.CodeInfo.Creator, code.CodesBytes, code.CodeInfo.Source, code.CodeInfo.Builder) if err != nil { - panic(err) + return sdkerrors.Wrapf(err, "code number %d", i) + } newInfo := keeper.GetCodeInfo(ctx, newId) if !bytes.Equal(code.CodeInfo.CodeHash, newInfo.CodeHash) { - panic("code hashes not same") + return sdkerrors.Wrap(types.ErrInvalid, "code hashes not same") } } - for _, contract := range data.Contracts { - keeper.importContract(ctx, contract.ContractAddress, &contract.ContractInfo, contract.ContractState) + for i, contract := range data.Contracts { + err := keeper.importContract(ctx, contract.ContractAddress, &contract.ContractInfo, contract.ContractState) + if err != nil { + return sdkerrors.Wrapf(err, "contract number %d", i) + } } - for _, seq := range data.Sequences { - keeper.importAutoIncrementID(ctx, seq.IDKey, seq.Value) + for i, seq := range data.Sequences { + err := keeper.importAutoIncrementID(ctx, seq.IDKey, seq.Value) + if err != nil { + return sdkerrors.Wrapf(err, "sequence number %d", i) + } } + return nil } // ExportGenesis returns a GenesisState for a given context and keeper. diff --git a/x/wasm/internal/keeper/genesis_test.go b/x/wasm/internal/keeper/genesis_test.go index 748135142a..0fedc631c9 100644 --- a/x/wasm/internal/keeper/genesis_test.go +++ b/x/wasm/internal/keeper/genesis_test.go @@ -124,8 +124,15 @@ func TestFailFastImport(t *testing.T) { }, expSuccess: true, }, - "prevent contracts that points to non existing codeID": { + "happy path: code info with two contracts": { src: types.GenesisState{ + Codes: []types.Code{{ + CodeInfo: wasmTypes.CodeInfo{ + CodeHash: codeHash[:], + Creator: anyAddress, + }, + CodesBytes: wasmCode, + }}, Contracts: []types.Contract{ { ContractAddress: addrFromUint64(1<<32 + 1), @@ -135,6 +142,30 @@ func TestFailFastImport(t *testing.T) { Label: "any", Created: &types.AbsoluteTxPosition{BlockHeight: 1, TxIndex: 1}, }, + }, { + ContractAddress: addrFromUint64(2<<32 + 1), + ContractInfo: wasmTypes.ContractInfo{ + CodeID: 1, + Creator: anyAddress, + Label: "any", + Created: &types.AbsoluteTxPosition{BlockHeight: 1, TxIndex: 1}, + }, + }, + }, + }, + expSuccess: true, + }, + "prevent contracts that points to non existing codeID": { + src: types.GenesisState{ + Contracts: []types.Contract{ + { + ContractAddress: contractAddress(1, 1), + ContractInfo: wasmTypes.ContractInfo{ + CodeID: 1, + Creator: anyAddress, + Label: "any", + Created: &types.AbsoluteTxPosition{BlockHeight: 1, TxIndex: 1}, + }, }, }, }, @@ -150,7 +181,7 @@ func TestFailFastImport(t *testing.T) { }}, Contracts: []types.Contract{ { - ContractAddress: addrFromUint64(1<<32 + 1), + ContractAddress: contractAddress(1, 1), ContractInfo: wasmTypes.ContractInfo{ CodeID: 1, Creator: anyAddress, @@ -158,7 +189,7 @@ func TestFailFastImport(t *testing.T) { Created: &types.AbsoluteTxPosition{BlockHeight: 1, TxIndex: 1}, }, }, { - ContractAddress: addrFromUint64(1<<32 + 1), + ContractAddress: contractAddress(1, 1), ContractInfo: wasmTypes.ContractInfo{ CodeID: 1, Creator: anyAddress, @@ -217,13 +248,12 @@ func TestFailFastImport(t *testing.T) { defer cleanup() require.NoError(t, types.ValidateGenesis(spec.src)) + got := InitGenesis(ctx, keeper, spec.src) if spec.expSuccess { - InitGenesis(ctx, keeper, spec.src) + require.NoError(t, got) return } - require.Panics(t, func() { - InitGenesis(ctx, keeper, spec.src) - }) + require.Error(t, got) }) } } diff --git a/x/wasm/internal/keeper/keeper.go b/x/wasm/internal/keeper/keeper.go index 74aa88eac6..cdf9701f3e 100644 --- a/x/wasm/internal/keeper/keeper.go +++ b/x/wasm/internal/keeper/keeper.go @@ -2,10 +2,10 @@ package keeper import ( "encoding/binary" - "fmt" "path/filepath" "github.com/cosmos/cosmos-sdk/x/staking" + "github.com/pkg/errors" wasm "github.com/CosmWasm/go-cosmwasm" wasmTypes "github.com/CosmWasm/go-cosmwasm/types" @@ -404,7 +404,7 @@ func (k Keeper) GetContractState(ctx sdk.Context, contractAddress sdk.AccAddress return prefixStore.Iterator(nil, nil) } -func (k Keeper) importContractState(ctx sdk.Context, contractAddress sdk.AccAddress, models []types.Model) { +func (k Keeper) importContractState(ctx sdk.Context, contractAddress sdk.AccAddress, models []types.Model) error { prefixStoreKey := types.GetContractStorePrefixKey(contractAddress) prefixStore := prefix.NewStore(ctx.KVStore(k.storeKey), prefixStoreKey) for _, model := range models { @@ -412,10 +412,11 @@ func (k Keeper) importContractState(ctx sdk.Context, contractAddress sdk.AccAddr model.Value = []byte{} } if prefixStore.Has(model.Key) { - panic(fmt.Sprintf("duplicate key: %x", model.Key)) + return sdkerrors.Wrapf(types.ErrDuplicate, "duplicate key: %x", model.Key) } prefixStore.Set(model.Key, model.Value) } + return nil } func (k Keeper) GetCodeInfo(ctx sdk.Context, codeID uint64) *types.CodeInfo { @@ -471,10 +472,15 @@ func consumeGas(ctx sdk.Context, gas uint64) { // generates a contract address from codeID + instanceID func (k Keeper) generateContractAddress(ctx sdk.Context, codeID uint64) sdk.AccAddress { instanceID := k.autoIncrementID(ctx, types.KeyLastInstanceID) + return contractAddress(codeID, instanceID) +} + +func contractAddress(codeID, instanceID uint64) sdk.AccAddress { // NOTE: It is possible to get a duplicate address if either codeID or instanceID // overflow 32 bits. This is highly improbable, but something that could be refactored. contractID := codeID<<32 + instanceID return addrFromUint64(contractID) + } func (k Keeper) GetNextCodeID(ctx sdk.Context) uint64 { @@ -510,24 +516,25 @@ func (k Keeper) peekAutoIncrementID(ctx sdk.Context, lastIDKey []byte) uint64 { return id } -func (k Keeper) importAutoIncrementID(ctx sdk.Context, lastIDKey []byte, val uint64) { +func (k Keeper) importAutoIncrementID(ctx sdk.Context, lastIDKey []byte, val uint64) error { store := ctx.KVStore(k.storeKey) if store.Has(lastIDKey) { - panic(fmt.Sprintf("duplicate autoincrement id: %s", string(lastIDKey))) + return sdkerrors.Wrapf(types.ErrDuplicate, "autoincrement id: %s", string(lastIDKey)) } bz := sdk.Uint64ToBigEndian(val) store.Set(lastIDKey, bz) + return nil } -func (k Keeper) importContract(ctx sdk.Context, address sdk.AccAddress, c *types.ContractInfo, state []types.Model) { +func (k Keeper) importContract(ctx sdk.Context, address sdk.AccAddress, c *types.ContractInfo, state []types.Model) error { if !k.containsCodeInfo(ctx, c.CodeID) { - panic(fmt.Sprintf("unknown code id: %d", c.CodeID)) + return errors.Wrapf(types.ErrNotFound, "code id: %d", c.CodeID) } if k.containsContractInfo(ctx, address) { - panic(fmt.Sprintf("duplicate contract: %s", address)) + return errors.Wrapf(types.ErrDuplicate, "contract: %s", address) } k.setContractInfo(ctx, address, c) - k.importContractState(ctx, address, state) + return k.importContractState(ctx, address, state) } func addrFromUint64(id uint64) sdk.AccAddress { diff --git a/x/wasm/internal/types/errors.go b/x/wasm/internal/types/errors.go index 3ab5b5c450..ad70bdabdb 100644 --- a/x/wasm/internal/types/errors.go +++ b/x/wasm/internal/types/errors.go @@ -46,4 +46,7 @@ var ( // ErrInvalid error for content that is invalid in this context ErrInvalid = sdkErrors.Register(DefaultCodespace, 13, "invalid") + + // ErrDuplicate error for content that exsists + ErrDuplicate = sdkErrors.Register(DefaultCodespace, 14, "duplicate") ) diff --git a/x/wasm/internal/types/genesis_test.go b/x/wasm/internal/types/genesis_test.go new file mode 100644 index 0000000000..5d84fc9db5 --- /dev/null +++ b/x/wasm/internal/types/genesis_test.go @@ -0,0 +1,107 @@ +package types + +import ( + "crypto/sha256" + "testing" + + "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/libs/rand" +) + +func TestValidateGenesisState(t *testing.T) { + specs := map[string]struct { + srcMutator func(state GenesisState) + expError bool + }{ + "all good": { + srcMutator: func(s GenesisState) {}, + }, + "codeinfo invalid": { + srcMutator: func(s GenesisState) { + s.Codes[0].CodeInfo.CodeHash = nil + }, + expError: true, + }, + "contract invalid": { + srcMutator: func(s GenesisState) { + s.Contracts[0].ContractAddress = nil + }, + expError: true, + }, + "sequence invalid": { + srcMutator: func(s GenesisState) { + s.Sequences[0].IDKey = nil + }, + expError: true, + }, + } + for msg, spec := range specs { + t.Run(msg, func(t *testing.T) { + state := genesisFixture(spec.srcMutator) + got := state.ValidateBasic() + if spec.expError { + require.Error(t, got) + return + } + require.NoError(t, got) + }) + } + +} + +func genesisFixture(mutators ...func(state GenesisState)) GenesisState { + const ( + numCodes = 2 + numContracts = 2 + numSequences = 2 + ) + + fixture := GenesisState{ + Codes: make([]Code, numCodes), + Contracts: make([]Contract, numContracts), + Sequences: make([]Sequence, numSequences), + } + for i := 0; i < numCodes; i++ { + fixture.Codes[i] = codeFixture() + } + for i := 0; i < numContracts; i++ { + fixture.Contracts[i] = contractFixture() + } + for i := 0; i < numSequences; i++ { + fixture.Sequences[i] = Sequence{ + IDKey: rand.Bytes(5), + Value: uint64(i), + } + } + for _, m := range mutators { + m(fixture) + } + return fixture +} + +func codeFixture() Code { + wasmCode := rand.Bytes(100) + codeHash := sha256.Sum256(wasmCode) + anyAddress := make([]byte, 20) + + return Code{ + CodeInfo: CodeInfo{ + CodeHash: codeHash[:], + Creator: anyAddress, + }, + CodesBytes: wasmCode, + } +} + +func contractFixture() Contract { + anyAddress := make([]byte, 20) + return Contract{ + ContractAddress: anyAddress, + ContractInfo: ContractInfo{ + CodeID: 1, + Creator: anyAddress, + Label: "any", + Created: &AbsoluteTxPosition{BlockHeight: 1, TxIndex: 1}, + }, + } +} diff --git a/x/wasm/internal/types/types.go b/x/wasm/internal/types/types.go index 1f6cbe10ef..896cf8e953 100644 --- a/x/wasm/internal/types/types.go +++ b/x/wasm/internal/types/types.go @@ -97,6 +97,12 @@ func (c *ContractInfo) ValidateBasic() error { if err := validateLabel(c.Label); err != nil { return sdkerrors.Wrap(err, "label") } + 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") + } return nil } @@ -119,6 +125,16 @@ 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 +} + // NewCreatedAt gets a timestamp from the context func NewCreatedAt(ctx sdk.Context) *AbsoluteTxPosition { // we must safely handle nil gas meters diff --git a/x/wasm/module.go b/x/wasm/module.go index e021516583..8964390348 100644 --- a/x/wasm/module.go +++ b/x/wasm/module.go @@ -114,7 +114,9 @@ func (am AppModule) NewQuerierHandler() sdk.Querier { func (am AppModule) InitGenesis(ctx sdk.Context, data json.RawMessage) []abci.ValidatorUpdate { var genesisState GenesisState ModuleCdc.MustUnmarshalJSON(data, &genesisState) - InitGenesis(ctx, am.keeper, genesisState) + if err := InitGenesis(ctx, am.keeper, genesisState); err != nil { + panic(err) + } return []abci.ValidatorUpdate{} } From 44696c4a98e117dc4a611b03105f40c7de0ed344 Mon Sep 17 00:00:00 2001 From: Alex Peters Date: Wed, 1 Jul 2020 09:59:40 +0200 Subject: [PATCH 3/3] Test validateBasic methods --- x/wasm/internal/keeper/genesis_test.go | 63 +++--------- x/wasm/internal/types/genesis_test.go | 137 +++++++++++++++---------- x/wasm/internal/types/test_fixtures.go | 102 ++++++++++++++++++ x/wasm/internal/types/types.go | 3 + x/wasm/internal/types/types_test.go | 133 ++++++++++++++++++++++++ 5 files changed, 332 insertions(+), 106 deletions(-) create mode 100644 x/wasm/internal/types/test_fixtures.go create mode 100644 x/wasm/internal/types/types_test.go diff --git a/x/wasm/internal/keeper/genesis_test.go b/x/wasm/internal/keeper/genesis_test.go index 0fedc631c9..b52cc3dcb4 100644 --- a/x/wasm/internal/keeper/genesis_test.go +++ b/x/wasm/internal/keeper/genesis_test.go @@ -101,7 +101,7 @@ func TestFailFastImport(t *testing.T) { }}, Contracts: nil, }}, - "happy path: code info and contract do match": { + "happy path: code id in info and contract do match": { src: types.GenesisState{ Codes: []types.Code{{ CodeInfo: wasmTypes.CodeInfo{ @@ -112,13 +112,8 @@ func TestFailFastImport(t *testing.T) { }}, Contracts: []types.Contract{ { - ContractAddress: addrFromUint64(1<<32 + 1), - ContractInfo: wasmTypes.ContractInfo{ - CodeID: 1, - Creator: anyAddress, - Label: "any", - Created: &types.AbsoluteTxPosition{BlockHeight: 1, TxIndex: 1}, - }, + ContractAddress: contractAddress(1, 1), + ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }), }, }, }, @@ -135,21 +130,11 @@ func TestFailFastImport(t *testing.T) { }}, Contracts: []types.Contract{ { - ContractAddress: addrFromUint64(1<<32 + 1), - ContractInfo: wasmTypes.ContractInfo{ - CodeID: 1, - Creator: anyAddress, - Label: "any", - Created: &types.AbsoluteTxPosition{BlockHeight: 1, TxIndex: 1}, - }, + ContractAddress: contractAddress(1, 1), + ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }), }, { - ContractAddress: addrFromUint64(2<<32 + 1), - ContractInfo: wasmTypes.ContractInfo{ - CodeID: 1, - Creator: anyAddress, - Label: "any", - Created: &types.AbsoluteTxPosition{BlockHeight: 1, TxIndex: 1}, - }, + ContractAddress: contractAddress(2, 1), + ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }), }, }, }, @@ -160,17 +145,12 @@ func TestFailFastImport(t *testing.T) { Contracts: []types.Contract{ { ContractAddress: contractAddress(1, 1), - ContractInfo: wasmTypes.ContractInfo{ - CodeID: 1, - Creator: anyAddress, - Label: "any", - Created: &types.AbsoluteTxPosition{BlockHeight: 1, TxIndex: 1}, - }, + ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }), }, }, }, }, - "prevent duplicate contracts": { + "prevent duplicate contract address": { src: types.GenesisState{ Codes: []types.Code{{ CodeInfo: wasmTypes.CodeInfo{ @@ -182,25 +162,15 @@ func TestFailFastImport(t *testing.T) { Contracts: []types.Contract{ { ContractAddress: contractAddress(1, 1), - ContractInfo: wasmTypes.ContractInfo{ - CodeID: 1, - Creator: anyAddress, - Label: "any", - Created: &types.AbsoluteTxPosition{BlockHeight: 1, TxIndex: 1}, - }, + ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }), }, { ContractAddress: contractAddress(1, 1), - ContractInfo: wasmTypes.ContractInfo{ - CodeID: 1, - Creator: anyAddress, - Label: "any", - Created: &types.AbsoluteTxPosition{BlockHeight: 1, TxIndex: 1}, - }, + ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }), }, }, }, }, - "prevent duplicate contract model": { + "prevent duplicate contract model keys": { src: types.GenesisState{ Codes: []types.Code{{ CodeInfo: wasmTypes.CodeInfo{ @@ -211,13 +181,8 @@ func TestFailFastImport(t *testing.T) { }}, Contracts: []types.Contract{ { - ContractAddress: addrFromUint64(1<<32 + 1), - ContractInfo: wasmTypes.ContractInfo{ - CodeID: 1, - Creator: anyAddress, - Label: "any", - Created: &types.AbsoluteTxPosition{BlockHeight: 1, TxIndex: 1}, - }, + ContractAddress: contractAddress(1, 1), + ContractInfo: types.ContractInfoFixture(func(c *wasmTypes.ContractInfo) { c.CodeID = 1 }), ContractState: []types.Model{ { Key: []byte{0x1}, diff --git a/x/wasm/internal/types/genesis_test.go b/x/wasm/internal/types/genesis_test.go index 5d84fc9db5..6314d07506 100644 --- a/x/wasm/internal/types/genesis_test.go +++ b/x/wasm/internal/types/genesis_test.go @@ -1,35 +1,34 @@ package types import ( - "crypto/sha256" + "bytes" "testing" "github.com/stretchr/testify/require" - "github.com/tendermint/tendermint/libs/rand" ) func TestValidateGenesisState(t *testing.T) { specs := map[string]struct { - srcMutator func(state GenesisState) + srcMutator func(*GenesisState) expError bool }{ "all good": { - srcMutator: func(s GenesisState) {}, + srcMutator: func(s *GenesisState) {}, }, "codeinfo invalid": { - srcMutator: func(s GenesisState) { + srcMutator: func(s *GenesisState) { s.Codes[0].CodeInfo.CodeHash = nil }, expError: true, }, "contract invalid": { - srcMutator: func(s GenesisState) { + srcMutator: func(s *GenesisState) { s.Contracts[0].ContractAddress = nil }, expError: true, }, "sequence invalid": { - srcMutator: func(s GenesisState) { + srcMutator: func(s *GenesisState) { s.Sequences[0].IDKey = nil }, expError: true, @@ -37,7 +36,7 @@ func TestValidateGenesisState(t *testing.T) { } for msg, spec := range specs { t.Run(msg, func(t *testing.T) { - state := genesisFixture(spec.srcMutator) + state := GenesisFixture(spec.srcMutator) got := state.ValidateBasic() if spec.expError { require.Error(t, got) @@ -46,62 +45,86 @@ func TestValidateGenesisState(t *testing.T) { require.NoError(t, got) }) } - } -func genesisFixture(mutators ...func(state GenesisState)) GenesisState { - const ( - numCodes = 2 - numContracts = 2 - numSequences = 2 - ) - - fixture := GenesisState{ - Codes: make([]Code, numCodes), - Contracts: make([]Contract, numContracts), - Sequences: make([]Sequence, numSequences), - } - for i := 0; i < numCodes; i++ { - fixture.Codes[i] = codeFixture() - } - for i := 0; i < numContracts; i++ { - fixture.Contracts[i] = contractFixture() - } - for i := 0; i < numSequences; i++ { - fixture.Sequences[i] = Sequence{ - IDKey: rand.Bytes(5), - Value: uint64(i), - } +func TestCodeValidateBasic(t *testing.T) { + specs := map[string]struct { + srcMutator func(*Code) + expError bool + }{ + "all good": {srcMutator: func(_ *Code) {}}, + "codeinfo invalid": { + srcMutator: func(c *Code) { + c.CodeInfo.CodeHash = nil + }, + expError: true, + }, + "codeBytes empty": { + srcMutator: func(c *Code) { + c.CodesBytes = []byte{} + }, + expError: true, + }, + "codeBytes nil": { + srcMutator: func(c *Code) { + c.CodesBytes = nil + }, + expError: true, + }, + "codeBytes greater limit": { + srcMutator: func(c *Code) { + c.CodesBytes = bytes.Repeat([]byte{0x1}, MaxWasmSize+1) + }, + expError: true, + }, } - for _, m := range mutators { - m(fixture) + for msg, spec := range specs { + t.Run(msg, func(t *testing.T) { + state := CodeFixture(spec.srcMutator) + got := state.ValidateBasic() + if spec.expError { + require.Error(t, got) + return + } + require.NoError(t, got) + }) } - return fixture } -func codeFixture() Code { - wasmCode := rand.Bytes(100) - codeHash := sha256.Sum256(wasmCode) - anyAddress := make([]byte, 20) - - return Code{ - CodeInfo: CodeInfo{ - CodeHash: codeHash[:], - Creator: anyAddress, +func TestContractValidateBasic(t *testing.T) { + specs := map[string]struct { + srcMutator func(*Contract) + expError bool + }{ + "all good": {srcMutator: func(_ *Contract) {}}, + "contract address invalid": { + srcMutator: func(c *Contract) { + c.ContractAddress = nil + }, + expError: true, }, - CodesBytes: wasmCode, - } -} - -func contractFixture() Contract { - anyAddress := make([]byte, 20) - return Contract{ - ContractAddress: anyAddress, - ContractInfo: ContractInfo{ - CodeID: 1, - Creator: anyAddress, - Label: "any", - Created: &AbsoluteTxPosition{BlockHeight: 1, TxIndex: 1}, + "contract info invalid": { + srcMutator: func(c *Contract) { + c.ContractInfo.Creator = nil + }, + expError: true, }, + "contract state invalid": { + srcMutator: func(c *Contract) { + c.ContractState = append(c.ContractState, Model{}) + }, + expError: true, + }, + } + for msg, spec := range specs { + t.Run(msg, func(t *testing.T) { + state := ContractFixture(spec.srcMutator) + got := state.ValidateBasic() + if spec.expError { + require.Error(t, got) + return + } + require.NoError(t, got) + }) } } diff --git a/x/wasm/internal/types/test_fixtures.go b/x/wasm/internal/types/test_fixtures.go new file mode 100644 index 0000000000..c78e1ddf68 --- /dev/null +++ b/x/wasm/internal/types/test_fixtures.go @@ -0,0 +1,102 @@ +package types + +import ( + "bytes" + "crypto/sha256" + + "github.com/tendermint/tendermint/libs/rand" +) + +func GenesisFixture(mutators ...func(*GenesisState)) GenesisState { + const ( + numCodes = 2 + numContracts = 2 + numSequences = 2 + ) + + fixture := GenesisState{ + Codes: make([]Code, numCodes), + Contracts: make([]Contract, numContracts), + Sequences: make([]Sequence, numSequences), + } + for i := 0; i < numCodes; i++ { + fixture.Codes[i] = CodeFixture() + } + for i := 0; i < numContracts; i++ { + fixture.Contracts[i] = ContractFixture() + } + for i := 0; i < numSequences; i++ { + fixture.Sequences[i] = Sequence{ + IDKey: rand.Bytes(5), + Value: uint64(i), + } + } + for _, m := range mutators { + m(&fixture) + } + return fixture +} + +func CodeFixture(mutators ...func(*Code)) Code { + wasmCode := rand.Bytes(100) + codeHash := sha256.Sum256(wasmCode) + anyAddress := make([]byte, 20) + + fixture := Code{ + CodeInfo: CodeInfo{ + CodeHash: codeHash[:], + Creator: anyAddress, + }, + CodesBytes: wasmCode, + } + + for _, m := range mutators { + m(&fixture) + } + return fixture +} + +func CodeInfoFixture(mutators ...func(*CodeInfo)) CodeInfo { + wasmCode := bytes.Repeat([]byte{0x1}, 10) + codeHash := sha256.Sum256(wasmCode) + anyAddress := make([]byte, 20) + fixture := CodeInfo{ + CodeHash: codeHash[:], + Creator: anyAddress, + Source: "https://example.com", + Builder: "my/builder:tag", + } + for _, m := range mutators { + m(&fixture) + } + return fixture +} + +func ContractFixture(mutators ...func(*Contract)) Contract { + anyAddress := make([]byte, 20) + fixture := Contract{ + ContractAddress: anyAddress, + ContractInfo: ContractInfoFixture(), + ContractState: []Model{{Key: []byte("anyKey"), Value: []byte("anyValue")}}, + } + + for _, m := range mutators { + m(&fixture) + } + return fixture +} + +func ContractInfoFixture(mutators ...func(*ContractInfo)) ContractInfo { + anyAddress := make([]byte, 20) + fixture := ContractInfo{ + CodeID: 1, + Creator: anyAddress, + Label: "any", + Created: &AbsoluteTxPosition{BlockHeight: 1, TxIndex: 1}, + } + + for _, m := range mutators { + m(&fixture) + } + return fixture +} diff --git a/x/wasm/internal/types/types.go b/x/wasm/internal/types/types.go index 896cf8e953..251c06a2af 100644 --- a/x/wasm/internal/types/types.go +++ b/x/wasm/internal/types/types.go @@ -97,6 +97,9 @@ 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") } diff --git a/x/wasm/internal/types/types_test.go b/x/wasm/internal/types/types_test.go new file mode 100644 index 0000000000..07498a0f98 --- /dev/null +++ b/x/wasm/internal/types/types_test.go @@ -0,0 +1,133 @@ +package types + +import ( + "strings" + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/require" +) + +func TestContractInfoValidateBasic(t *testing.T) { + specs := map[string]struct { + srcMutator func(*ContractInfo) + expError bool + }{ + "all good": {srcMutator: func(_ *ContractInfo) {}}, + "code id empty": { + srcMutator: func(c *ContractInfo) { c.CodeID = 0 }, + expError: true, + }, + "creator empty": { + srcMutator: func(c *ContractInfo) { c.Creator = nil }, + expError: true, + }, + "creator not an address": { + srcMutator: func(c *ContractInfo) { c.Creator = make([]byte, sdk.AddrLen-1) }, + expError: true, + }, + "admin empty": { + srcMutator: func(c *ContractInfo) { c.Admin = nil }, + expError: false, + }, + "admin not an address": { + srcMutator: func(c *ContractInfo) { c.Admin = make([]byte, sdk.AddrLen-1) }, + expError: true, + }, + "label empty": { + srcMutator: func(c *ContractInfo) { c.Label = "" }, + expError: true, + }, + "label exceeds limit": { + 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, + }, + "created invalid": { + 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) { + state := ContractInfoFixture(spec.srcMutator) + got := state.ValidateBasic() + if spec.expError { + require.Error(t, got) + return + } + require.NoError(t, got) + }) + } +} + +func TestCodeInfoValidateBasic(t *testing.T) { + specs := map[string]struct { + srcMutator func(*CodeInfo) + expError bool + }{ + "all good": {srcMutator: func(_ *CodeInfo) {}}, + "code hash empty": { + srcMutator: func(c *CodeInfo) { c.CodeHash = []byte{} }, + expError: true, + }, + "code hash nil": { + srcMutator: func(c *CodeInfo) { c.CodeHash = nil }, + expError: true, + }, + "creator empty": { + srcMutator: func(c *CodeInfo) { c.Creator = nil }, + expError: true, + }, + "creator not an address": { + srcMutator: func(c *CodeInfo) { c.Creator = make([]byte, sdk.AddrLen-1) }, + expError: true, + }, + "source empty": { + srcMutator: func(c *CodeInfo) { c.Source = "" }, + }, + "source not an url": { + srcMutator: func(c *CodeInfo) { c.Source = "invalid" }, + expError: true, + }, + "source not an absolute url": { + srcMutator: func(c *CodeInfo) { c.Source = "../bar.txt" }, + expError: true, + }, + "source not https schema url": { + srcMutator: func(c *CodeInfo) { c.Source = "http://example.com" }, + expError: true, + }, + "builder tag exceeds limit": { + srcMutator: func(c *CodeInfo) { c.Builder = strings.Repeat("a", MaxBuildTagSize+1) }, + expError: true, + }, + "builder tag does not match pattern": { + srcMutator: func(c *CodeInfo) { c.Builder = "invalid" }, + expError: true, + }, + } + for msg, spec := range specs { + t.Run(msg, func(t *testing.T) { + state := CodeInfoFixture(spec.srcMutator) + got := state.ValidateBasic() + if spec.expError { + require.Error(t, got) + return + } + require.NoError(t, got) + }) + } +}