diff --git a/cosmwasm/contracts/v1/compute-tests/test-compute-contract/src/contract.rs b/cosmwasm/contracts/v1/compute-tests/test-compute-contract/src/contract.rs index 5cb10525c..050f09e36 100644 --- a/cosmwasm/contracts/v1/compute-tests/test-compute-contract/src/contract.rs +++ b/cosmwasm/contracts/v1/compute-tests/test-compute-contract/src/contract.rs @@ -669,24 +669,18 @@ pub fn execute(deps: DepsMut, env: Env, info: MessageInfo, msg: ExecuteMsg) -> S let mut response = Response::default(); - // add_succeeding_submsg(env, &mut response, ReplyOn::Always, 9201); - - // response = response.add_message( - // CosmosMsg::Bank(BankMsg::Send { - // to_address: "non-existent".to_string(), - // amount: vec![Coin::new(100, "non-existent")], - // }) - // ); + add_succeeding_submsg(env, &mut response, ReplyOn::Always, 9201); response = response.add_message( - CosmosMsg::Wasm(WasmMsg::Execute { - contract_addr: env.contract.address.clone().into_string(), - code_hash: env.contract.code_hash.clone(), - msg: Binary::from(r#"{"quick_error":{}}"#.as_bytes().to_vec()), - funds: vec![], + CosmosMsg::Bank(BankMsg::Send { + to_address: "non-existent".to_string(), + amount: vec![Coin::new(100, "non-existent")], }) ); + // NOTE that if the submsg is added after the message, then it is processed in THAT order, + // which is different from what is stated in cosmwasm's docs here: https://github.com/CosmWasm/cosmwasm/blob/main/SEMANTICS.md#order-and-rollback + Ok(response) } ExecuteMsg::InitV10 { diff --git a/x/compute/internal/keeper/secret_contracts_submsgs_test.go b/x/compute/internal/keeper/secret_contracts_submsgs_test.go index a7a3f3e2f..8bee29a99 100644 --- a/x/compute/internal/keeper/secret_contracts_submsgs_test.go +++ b/x/compute/internal/keeper/secret_contracts_submsgs_test.go @@ -109,13 +109,13 @@ Test case: Note: If the reply was called, it would have incremented the counter again (to 12) Observed Outcome: + - Contract does not handle reply, even though it is reply_on=always, because sdk messages revert the whole tx. This is because regular messages are not postponed until all Submsgs are processed. Instead, they are processed in the same ordering as SubMsgs. This differs from cosmwasm's documentation at https://github.com/CosmWasm/cosmwasm/blob/main/SEMANTICS.md#order-and-rollback - The difference in outcomes is small when sdk message fails, because either way, the whole tx is reverted, but may - be present in cases of contract-queries-to-chain if the sdk message succeeds. - Counter is still on 11. SubMsg changes reverted, reply not reached. (first increment remains as it is reverted in sdk) + - Error is not empty, whole tx will revert because of sdk message failing. */ func TestV1StateRevertsAfterSubmessageThatGeneratesBankMsgFails(t *testing.T) { @@ -143,20 +143,21 @@ Test case: 0. Initial counter stored in state: 10 1. Contract increments 1 (=11) 2. Contract sends: - 2a. Submessage to itself: + 2a. Submessage to itself (reply=always): 2a-1. Submessage execution increments counter by 3 (=14) 2a-2. Submessage succeeds + 2a-3. Contract handles reply, increments counter by 1 (=15) 2b. Failing Bank message. Expected Result: - Error is not empty - will revert all changes - - Counter is on 11 (main message revert happens in sdk, not reflected here) + - Counter is on 15 (main message revert happens in sdk, not reflected here) */ func TestV1SubmessageStateRevertsIfCallerFails(t *testing.T) { ctx, keeper, codeID, _, walletA, privKeyA, _, _ := setupTest(t, TestContractPaths[v1Contract], sdk.NewCoins()) _, _, contractAddress, _, _ := initHelper(t, keeper, ctx, codeID, walletA, nil, privKeyA, `{"counter":{"counter":10, "expires":100}}`, true, true, defaultGasForTests) - _, _, _, _, _, err := execHelper(t, keeper, ctx, contractAddress, walletA, privKeyA, `{"send_succeeding_submessage_and_failing_message":{}}`, true, true, math.MaxUint64, 0) + _, _, _, _, _, err := execHelper(t, keeper, ctx, contractAddress, walletA, privKeyA, `{"send_succeeding_submessage_and_failing_message":{}}`, false, true, math.MaxUint64, 0) require.NotEmpty(t, err) @@ -169,7 +170,7 @@ func TestV1SubmessageStateRevertsIfCallerFails(t *testing.T) { // The state here should have been reverted by the APP but in go-tests we create our own keeper so it is not reverted // in this case. Only the submessage changes revert, since we manage them in a sub-context in our keeper. - require.Equal(t, uint32(11), resp.Get.Count) + require.Equal(t, uint32(15), resp.Get.Count) } /*