Don't error whole transaction if callback receiver errors

[0xekez]

To avoid packets that are stuck in the queue, we shouldn't error the transaction if a callback receiver errors. This requires two changes:

1. reply_on_error for callbacks and don't do anything in response to the error.
2. gas limits on callback execution. see https://github.com/DA0-DA0/polytone/pull/22 which solves the same problem for ACKs.

[Art3miX]

Just a question, what gas limit actually do in case of reply_on_error? So if the callback reach out of gas, and we execute it as reply_on_error, and do not set a gas limit, will it fail the whole ACK tx? or just act as a normal reply_on_error (only fail the callback, and not the whole messages tree)?

If the later is true we don't actually need to do No.2, because it should be handled for us by the SDK.

Which should be the logical case here, if we used X gas so far, and the limit is 2X gas, but the callback need 2X gas, and we set reply_on_error, then it should only fail the callback, and continue. as always the docs doesn't help: https://github.com/CosmWasm/cosmwasm/blob/main/docs/GAS.md

[0xekez]

what gas limit actually do in case of reply_on_error?

good question. I am 99% sure that if you run out of gas inside a reply_on_error transaction the whole thing gets rolled back and your reply handler gets ignored. My intuition as to why this is the case is that otherwise you could repeatedly run out of gas, catch the error, run out of gas, etc.

we should see about getting to the bottom of this with some tests.

[0xekez]

I think the change here is:

1. Move reply_on_error into the callback package.
2. Determine the amount of gas that needs to be saved for callback and use that in combination from the BLOCK_MAX_GAS to save enough gas for the callback handling. This will involve adding a configurable BLOCK_MAX_GAS parameter to the Note module.

For testing and determining the amount of gas to save, this comment may be helpful for guidance in doing that with the simulation tests:

https://github.com/DA0-DA0/polytone/blob/d4b1793b0c4c7ed735ee6bd1323812b371a6887f/tests/simtests/functionality_suite.go#L39-L44

[0xekez]

We should probably move the reply method into the callback package as well. Only change I'd make is taking the sequence as an argument, so the new function would look something like this:

pub fn reply(deps: DepsMut, _env: Env, msg: Reply) -> Result<Response, ContractError> {
    let sequence = msg.id;
    callback::on_reply(deps.storage, sequence, msg)
}

The advantage of splitting it up like that is that we can re-use this package later in the dao-contracts using tagged reply IDs.

[Art3miX]

good question. I am 99% sure that if you run out of gas inside a reply_on_error transaction the whole thing gets rolled back and your reply handler gets ignored.

Reading this: https://github.com/CosmWasm/cosmwasm/pull/1646/files#diff-f9839d73197185aaec052064f43a324bd9309413f3ad36183c3247580b1b6669R80

If i understand it correctly, even if there is a VM error ('out of gas' for our case) it will not fail the whole TX, but rather just fail the out of gas msg, and will return reply

Does that mean we can avoid using manual gas calculation/limits in our case? and if so, what is the use-case of gas-limit?

[0xekez]

https://github.com/DA0-DA0/polytone/commit/9733a273ecf84c46c9e51e2bdc2ed00c8347f9cd