-
Notifications
You must be signed in to change notification settings - Fork 195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add validation for multi message execution wasm #2092
Changes from 3 commits
236c9d7
0b68c73
5174c5e
4985ff6
ae0f2d0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test cases look good |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ | |
wasm "github.com/CosmWasm/wasmd/x/wasm/types" | ||
|
||
"github.com/NibiruChain/nibiru/v2/x/common/testutil" | ||
"github.com/NibiruChain/nibiru/v2/x/common/testutil/testapp" | ||
"github.com/NibiruChain/nibiru/v2/x/evm/embeds" | ||
"github.com/NibiruChain/nibiru/v2/x/evm/evmtest" | ||
"github.com/NibiruChain/nibiru/v2/x/evm/precompile" | ||
|
@@ -313,3 +314,188 @@ | |
}) | ||
} | ||
} | ||
|
||
type WasmExecuteMsg struct { | ||
ContractAddr string `json:"contractAddr"` | ||
MsgArgs []byte `json:"msgArgs"` | ||
Funds []precompile.WasmBankCoin `json:"funds"` | ||
} | ||
Unique-Divine marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
func (s *WasmSuite) TestExecuteMultiValidation() { | ||
deps := evmtest.NewTestDeps() | ||
|
||
s.Require().NoError(testapp.FundAccount( | ||
deps.App.BankKeeper, | ||
deps.Ctx, | ||
deps.Sender.NibiruAddr, | ||
sdk.NewCoins(sdk.NewCoin("unibi", sdk.NewInt(100))), | ||
)) | ||
|
||
wasmContracts := SetupWasmContracts(&deps, &s.Suite) | ||
Check failure on line 334 in x/evm/precompile/wasm_test.go
|
||
Unique-Divine marked this conversation as resolved.
Show resolved
Hide resolved
|
||
wasmContract := wasmContracts[1] // hello_world_counter.wasm | ||
|
||
invalidMsgArgsBz := []byte(`{"invalid": "json"}`) // Invalid message format | ||
validMsgArgsBz := []byte(`{"increment": {}}`) // Valid increment message | ||
|
||
var emptyFunds []precompile.WasmBankCoin | ||
validFunds := []precompile.WasmBankCoin{{ | ||
Denom: "unibi", | ||
Amount: big.NewInt(100), | ||
}} | ||
invalidFunds := []precompile.WasmBankCoin{{ | ||
Denom: "invalid!denom", | ||
Amount: big.NewInt(100), | ||
}} | ||
|
||
testCases := []struct { | ||
name string | ||
executeMsgs []WasmExecuteMsg | ||
wantError string | ||
}{ | ||
{ | ||
name: "valid - single message", | ||
executeMsgs: []WasmExecuteMsg{ | ||
{ | ||
ContractAddr: wasmContract.String(), | ||
MsgArgs: validMsgArgsBz, | ||
Funds: emptyFunds, | ||
}, | ||
}, | ||
wantError: "", | ||
}, | ||
{ | ||
name: "valid - multiple messages", | ||
executeMsgs: []WasmExecuteMsg{ | ||
{ | ||
ContractAddr: wasmContract.String(), | ||
MsgArgs: validMsgArgsBz, | ||
Funds: validFunds, | ||
}, | ||
{ | ||
ContractAddr: wasmContract.String(), | ||
MsgArgs: validMsgArgsBz, | ||
Funds: emptyFunds, | ||
}, | ||
}, | ||
wantError: "", | ||
}, | ||
{ | ||
name: "invalid - malformed contract address", | ||
executeMsgs: []WasmExecuteMsg{ | ||
{ | ||
ContractAddr: "invalid-address", | ||
MsgArgs: validMsgArgsBz, | ||
Funds: emptyFunds, | ||
}, | ||
}, | ||
wantError: "decoding bech32 failed", | ||
}, | ||
{ | ||
name: "invalid - malformed message args", | ||
executeMsgs: []WasmExecuteMsg{ | ||
{ | ||
ContractAddr: wasmContract.String(), | ||
MsgArgs: invalidMsgArgsBz, | ||
Funds: emptyFunds, | ||
}, | ||
}, | ||
wantError: "unknown variant", | ||
}, | ||
{ | ||
name: "invalid - malformed funds", | ||
executeMsgs: []WasmExecuteMsg{ | ||
{ | ||
ContractAddr: wasmContract.String(), | ||
MsgArgs: validMsgArgsBz, | ||
Funds: invalidFunds, | ||
}, | ||
}, | ||
wantError: "invalid coins", | ||
}, | ||
{ | ||
name: "invalid - second message fails validation", | ||
executeMsgs: []WasmExecuteMsg{ | ||
{ | ||
ContractAddr: wasmContract.String(), | ||
MsgArgs: validMsgArgsBz, | ||
Funds: emptyFunds, | ||
}, | ||
{ | ||
ContractAddr: wasmContract.String(), | ||
MsgArgs: invalidMsgArgsBz, | ||
Funds: emptyFunds, | ||
}, | ||
}, | ||
wantError: "unknown variant", | ||
}, | ||
} | ||
|
||
for _, tc := range testCases { | ||
s.Run(tc.name, func() { | ||
callArgs := []any{tc.executeMsgs} | ||
input, err := embeds.SmartContract_Wasm.ABI.Pack( | ||
string(precompile.WasmMethod_executeMulti), | ||
callArgs..., | ||
) | ||
s.Require().NoError(err) | ||
|
||
ethTxResp, err := deps.EvmKeeper.CallContractWithInput( | ||
Check failure on line 442 in x/evm/precompile/wasm_test.go
|
||
deps.Ctx, deps.Sender.EthAddr, &precompile.PrecompileAddr_Wasm, true, input, | ||
) | ||
Unique-Divine marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if tc.wantError != "" { | ||
s.Require().Error(err) | ||
s.Require().Contains(err.Error(), tc.wantError) | ||
s.Require().Nil(ethTxResp) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve error assertions by checking error types instead of error messages Comparing error messages using substrings can be brittle and may lead to fragile tests. Consider checking for specific error types or using error wrapping to make your tests more robust and maintainable. |
||
} else { | ||
s.Require().NoError(err) | ||
s.Require().NotNil(ethTxResp) | ||
s.Require().NotEmpty(ethTxResp.Ret) | ||
} | ||
}) | ||
} | ||
} | ||
Unique-Divine marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// TestExecuteMultiPartialExecution ensures that no state changes occur if any message | ||
// in the batch fails validation | ||
func (s *WasmSuite) TestExecuteMultiPartialExecution() { | ||
deps := evmtest.NewTestDeps() | ||
wasmContracts := SetupWasmContracts(&deps, &s.Suite) | ||
Check failure on line 463 in x/evm/precompile/wasm_test.go
|
||
Unique-Divine marked this conversation as resolved.
Show resolved
Hide resolved
|
||
wasmContract := wasmContracts[1] // hello_world_counter.wasm | ||
|
||
// First verify initial state is 0 | ||
s.assertWasmCounterState(deps, wasmContract, 0) | ||
Check failure on line 467 in x/evm/precompile/wasm_test.go
|
||
Unique-Divine marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Create a batch where the second message will fail validation | ||
executeMsgs := []WasmExecuteMsg{ | ||
{ | ||
ContractAddr: wasmContract.String(), | ||
MsgArgs: []byte(`{"increment": {}}`), | ||
Funds: []precompile.WasmBankCoin{}, | ||
}, | ||
{ | ||
ContractAddr: wasmContract.String(), | ||
MsgArgs: []byte(`{"invalid": "json"}`), // This will fail validation | ||
Funds: []precompile.WasmBankCoin{}, | ||
}, | ||
} | ||
|
||
callArgs := []any{executeMsgs} | ||
input, err := embeds.SmartContract_Wasm.ABI.Pack( | ||
string(precompile.WasmMethod_executeMulti), | ||
callArgs..., | ||
) | ||
s.Require().NoError(err) | ||
|
||
ethTxResp, err := deps.EvmKeeper.CallContractWithInput( | ||
Check failure on line 490 in x/evm/precompile/wasm_test.go
|
||
deps.Ctx, deps.Sender.EthAddr, &precompile.PrecompileAddr_Wasm, true, input, | ||
) | ||
Unique-Divine marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Verify that the call failed | ||
s.Require().Error(err) | ||
s.Require().Contains(err.Error(), "unknown variant") | ||
s.Require().Nil(ethTxResp) | ||
|
||
// Verify that no state changes occurred | ||
s.assertWasmCounterState(deps, wasmContract, 0) | ||
Check failure on line 500 in x/evm/precompile/wasm_test.go
|
||
} | ||
Unique-Divine marked this conversation as resolved.
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance validation by collecting all errors before returning
Currently, the validation loop returns upon encountering the first error. Collecting all validation errors before returning could provide more comprehensive feedback to the caller.
You might modify the code as follows:
📝 Committable suggestion