From 2549e7222e5849db4812f696ec0f3189664bb350 Mon Sep 17 00:00:00 2001 From: Christoph Otter Date: Thu, 1 Feb 2024 18:39:09 +0100 Subject: [PATCH 1/4] Add Payload fields --- types/submessages.go | 34 ++++++++++++++++++++++++++++++---- types/submessages_test.go | 3 ++- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/types/submessages.go b/types/submessages.go index faaa14a53..4965027d2 100644 --- a/types/submessages.go +++ b/types/submessages.go @@ -55,10 +55,29 @@ func (s *replyOn) UnmarshalJSON(b []byte) error { // SubMsg wraps a CosmosMsg with some metadata for handling replies (ID) and optionally // limiting the gas usage (GasLimit) type SubMsg struct { - ID uint64 `json:"id"` - Msg CosmosMsg `json:"msg"` - GasLimit *uint64 `json:"gas_limit,omitempty"` - ReplyOn replyOn `json:"reply_on"` + // An arbitrary ID chosen by the contract. + // This is typically used to match `Reply`s in the `reply` entry point to the submessage. + ID uint64 `json:"id"` + Msg CosmosMsg `json:"msg"` + // Some arbitrary data that the contract can set in an application specific way. + // This is just passed into the `reply` entry point and is not stored to state. + // Any encoding can be used. If `id` is used to identify a particular action, + // the encoding can also be different for each of those actions since you can match `id` + // first and then start processing the `payload`. + // + // The environment restricts the length of this field in order to avoid abuse. The limit + // is environment specific and can change over time. The initial default is 128 KiB. + // + // Unset/nil/null cannot be differentiated from empty data. + // + // On chains running CosmWasm 1.x this field will be ignored. + Payload []byte `json:"payload"` + // Gas limit measured in [Cosmos SDK gas](https://github.com/CosmWasm/cosmwasm/blob/main/docs/GAS.md). + // + // Setting this to `None` means unlimited. Then the submessage execution can consume all gas of + // the current execution context. + GasLimit *uint64 `json:"gas_limit,omitempty"` + ReplyOn replyOn `json:"reply_on"` } // The result object returned to `reply`. We always get the ID from the submessage back and then must handle success and error cases ourselves. @@ -68,6 +87,13 @@ type Reply struct { // The ID that the contract set when emitting the `SubMsg`. Use this to identify which submessage triggered the `reply`. ID uint64 `json:"id"` Result SubMsgResult `json:"result"` + // Some arbitrary data that the contract set when emitting the `SubMsg`. + // This is just passed into the `reply` entry point and is not stored to state. + // + // Unset/nil/null cannot be differentiated from empty data. + // + // On chains running CosmWasm 1.x this field is never filled. + Payload []byte `json:"payload"` } // SubMsgResult is the raw response we return from wasmd after executing a SubMsg. diff --git a/types/submessages_test.go b/types/submessages_test.go index 0c9ab5401..1841ad591 100644 --- a/types/submessages_test.go +++ b/types/submessages_test.go @@ -33,10 +33,11 @@ func TestReplySerialization(t *testing.T) { }, }, }, + Payload: []byte("payload"), } serialized, err := json.Marshal(&reply1) require.NoError(t, err) - require.Equal(t, `{"gas_used":4312324,"id":75,"result":{"ok":{"events":[{"type":"hi","attributes":[{"key":"si","value":"claro"}]}],"data":"PwCqXKs=","msg_responses":[{"type_url":"/cosmos.bank.v1beta1.MsgSendResponse","value":""}]}}}`, string(serialized)) + require.Equal(t, `{"gas_used":4312324,"id":75,"result":{"ok":{"events":[{"type":"hi","attributes":[{"key":"si","value":"claro"}]}],"data":"PwCqXKs=","msg_responses":[{"type_url":"/cosmos.bank.v1beta1.MsgSendResponse","value":""}]}},"payload":"cGF5bG9hZA=="}`, string(serialized)) } func TestSubMsgResponseSerialization(t *testing.T) { From e633132d85482145a7e2dfd0f2e815e20edc3e96 Mon Sep 17 00:00:00 2001 From: Christoph Otter Date: Mon, 5 Feb 2024 16:41:22 +0100 Subject: [PATCH 2/4] Add payload length validation --- lib.go | 16 ++++++++++++++++ lib_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ types/ibc.go | 14 ++++++++++++++ types/msg.go | 7 +++++++ 4 files changed, 79 insertions(+) diff --git a/lib.go b/lib.go index 701ddb05a..8a0dde4d2 100644 --- a/lib.go +++ b/lib.go @@ -540,6 +540,11 @@ func compileCost(code WasmCode) uint64 { return CostPerByte * uint64(len(code)) } +// hasSubMessages is an interface for contract results that can contain sub-messages. +type hasSubMessages interface { + SubMessages() []types.SubMsg +} + func DeserializeResponse(gasLimit uint64, deserCost types.UFraction, gasReport *types.GasReport, data []byte, response any) error { gasForDeserialization := deserCost.Mul(uint64(len(data))).Floor() if gasLimit < gasForDeserialization+gasReport.UsedInternally { @@ -553,5 +558,16 @@ func DeserializeResponse(gasLimit uint64, deserCost types.UFraction, gasReport * return err } + // All responses that have sub-messages need their payload size to be checked + const ReplyPayloadMaxBytes = 128 * 1024 // 128 KiB + if response, ok := response.(hasSubMessages); ok { + for _, m := range response.SubMessages() { + // each payload needs to be below maximum size + if len(m.Payload) > ReplyPayloadMaxBytes { + return fmt.Errorf("reply payload larger than %d bytes: %d bytes", ReplyPayloadMaxBytes, len(m.Payload)) + } + } + } + return nil } diff --git a/lib_test.go b/lib_test.go index 4d7a4c957..d21b95fb5 100644 --- a/lib_test.go +++ b/lib_test.go @@ -4,6 +4,8 @@ package cosmwasm import ( "encoding/json" + "fmt" + "math" "os" "testing" @@ -368,3 +370,43 @@ func TestGetMetrics(t *testing.T) { require.Equal(t, uint64(0), metrics.SizePinnedMemoryCache) require.InEpsilon(t, 2832576, metrics.SizeMemoryCache, 0.25) } + +func TestLongPayloadDeserialization(t *testing.T) { + deserCost := types.UFraction{Numerator: 1, Denominator: 1} + gasReport := types.GasReport{} + + // Create a valid payload + validPayload := make([]byte, 128*1024) + validPayloadJSON, err := json.Marshal(validPayload) + require.NoError(t, err) + resultJson := []byte(fmt.Sprintf(`{"ok":{"messages":[{"id":0,"msg":{"bank":{"send":{"to_address":"bob","amount":[{"denom":"ATOM","amount":"250"}]}}},"payload":%s,"reply_on":"never"}],"data":"8Auq","attributes":[],"events":[]}}`, validPayloadJSON)) + + // Test that a valid payload can be deserialized + var result types.ContractResult + err = DeserializeResponse(math.MaxUint64, deserCost, &gasReport, resultJson, &result) + require.NoError(t, err) + require.Equal(t, validPayload, result.Ok.Messages[0].Payload) + + // Create an invalid payload (too large) + invalidPayload := make([]byte, 128*1024+1) + invalidPayloadJSON, err := json.Marshal(invalidPayload) + require.NoError(t, err) + resultJson = []byte(fmt.Sprintf(`{"ok":{"messages":[{"id":0,"msg":{"bank":{"send":{"to_address":"bob","amount":[{"denom":"ATOM","amount":"250"}]}}},"payload":%s,"reply_on":"never"}],"attributes":[],"events":[]}}`, invalidPayloadJSON)) + + // Test that an invalid payload cannot be deserialized + err = DeserializeResponse(math.MaxUint64, deserCost, &gasReport, resultJson, &result) + require.Error(t, err) + require.Contains(t, err.Error(), "payload") + + // Test that an invalid payload cannot be deserialized to IBCBasicResult + var ibcResult types.IBCBasicResult + err = DeserializeResponse(math.MaxUint64, deserCost, &gasReport, resultJson, &ibcResult) + require.Error(t, err) + require.Contains(t, err.Error(), "payload") + + // Test that an invalid payload cannot be deserialized to IBCReceiveResult + var ibcReceiveResult types.IBCReceiveResult + err = DeserializeResponse(math.MaxUint64, deserCost, &gasReport, resultJson, &ibcReceiveResult) + require.Error(t, err) + require.Contains(t, err.Error(), "payload") +} diff --git a/types/ibc.go b/types/ibc.go index eba4d3e9f..1e2b4b223 100644 --- a/types/ibc.go +++ b/types/ibc.go @@ -222,6 +222,13 @@ type IBCBasicResult struct { Err string `json:"error,omitempty"` } +func (r *IBCBasicResult) SubMessages() []SubMsg { + if r.Ok != nil { + return r.Ok.Messages + } + return nil +} + // IBCBasicResponse defines the return value on a successful processing. // This is the counterpart of [IbcBasicResponse](https://github.com/CosmWasm/cosmwasm/blob/v0.14.0-beta1/packages/std/src/ibc.rs#L194-L216). type IBCBasicResponse struct { @@ -249,6 +256,13 @@ type IBCReceiveResult struct { Err string `json:"error,omitempty"` } +func (r *IBCReceiveResult) SubMessages() []SubMsg { + if r.Ok != nil { + return r.Ok.Messages + } + return nil +} + // IBCReceiveResponse defines the return value on packet response processing. // This "success" case should be returned even in application-level errors, // Where the Acknowledgement bytes contain an encoded error message to be returned to diff --git a/types/msg.go b/types/msg.go index 016faaea0..3bae8e6da 100644 --- a/types/msg.go +++ b/types/msg.go @@ -14,6 +14,13 @@ type ContractResult struct { Err string `json:"error,omitempty"` } +func (r *ContractResult) SubMessages() []SubMsg { + if r.Ok != nil { + return r.Ok.Messages + } + return nil +} + // Response defines the return value on a successful instantiate/execute/migrate. // This is the counterpart of [Response](https://github.com/CosmWasm/cosmwasm/blob/v0.14.0-beta1/packages/std/src/results/response.rs#L73-L88) type Response struct { From edc91af1a2cf3bb008b695529af1f67fc68bc54b Mon Sep 17 00:00:00 2001 From: Christoph Otter Date: Mon, 5 Feb 2024 16:59:00 +0100 Subject: [PATCH 3/4] Add migrating entry --- docs/MIGRATING.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/MIGRATING.md b/docs/MIGRATING.md index 95d4bcdec..e40405a3e 100644 --- a/docs/MIGRATING.md +++ b/docs/MIGRATING.md @@ -42,6 +42,9 @@ the old behavior is that the new type will unmarshal to an empty slice when the JSON value is `null` or `[]`. Previously, both cases resulted in a `nil` value. +- `SubMsg` and `Reply` now have a new `Payload` field. This contains arbitrary + bytes from the contract that should be passed through to the corresponding + `Reply` call. ## Renamings From 9f9e27edf6748fed6e664fc72e7efc4b6d514401 Mon Sep 17 00:00:00 2001 From: Christoph Otter Date: Mon, 5 Feb 2024 17:02:27 +0100 Subject: [PATCH 4/4] Improve error message --- lib.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib.go b/lib.go index 8a0dde4d2..60a0498c2 100644 --- a/lib.go +++ b/lib.go @@ -561,10 +561,10 @@ func DeserializeResponse(gasLimit uint64, deserCost types.UFraction, gasReport * // All responses that have sub-messages need their payload size to be checked const ReplyPayloadMaxBytes = 128 * 1024 // 128 KiB if response, ok := response.(hasSubMessages); ok { - for _, m := range response.SubMessages() { + for i, m := range response.SubMessages() { // each payload needs to be below maximum size if len(m.Payload) > ReplyPayloadMaxBytes { - return fmt.Errorf("reply payload larger than %d bytes: %d bytes", ReplyPayloadMaxBytes, len(m.Payload)) + return fmt.Errorf("reply contains submessage at index %d with payload larger than %d bytes: %d bytes", i, ReplyPayloadMaxBytes, len(m.Payload)) } } }