You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently any revert in the external contract call of the application callback will revert the entire transaction. This will lead to suboptimal behavior and even errors in our transaction callbacks.
In OnSendPacket, the behavior is correct. If a revert happens in the callback here, we don't want to send the packet at all so we should revert everything.
In OnRecvPacket, any revert inside the app callback will revert the entire tx. This isn't incorrect but suboptimal. Here we will not have an error ack but instead will have to rely entirely on timeouts which will extend the time taken before tokens are reimbursed in the case of ICS20. Moreover, it will allow relayers to continue submitting a recvPacket tx even though it will never succeed. In particular, the app developers are highly prone to error since they must be careful to not create partial state changes and then return an error ack.
In OnAck/OnTimeoutPacket, any revert inside the app callback will revert the entire tx. This is suboptimal in the single payload case and wrong in the multipayload case since a single revert in a single app will prevent packet lifecycle completion for every other app. Moreover, we again have inefficiencies with the relayer since they may continue resubmission.
Solution:
Use try/catch for the app callbacks. For OnAck/OnTimeout callbacks, we just ignore the app error and revert the app-level changes while still committting the overall tx. This fixes the relayer issue, and in multipayload we will correctly execute all app callbacks regardless in one app errors.
For OnRecvPacket, we will also wrap in a try/catch. Any error that occurs we will convert into an error ack and add to our acknowledgement list. This correctly reverts the app state if an error is returned while still writing an error ack. This makes writing applications correctly much easier since you do not have to worry about reverting state changes yourself. However, in order to do this with Solidity I needed to create an error acknowledgment in the core handler.
This was difficult to do with the current design since the acknowledgements are completely up to the application. So there was no way that IBC core handler could construct an error ack from an error message since there was no standardized way to describe an error acknowledgment.
This led to prefixing the error coming from the app callback with the boolean false to create a standardized error acknowledgment. Thus the success acknowledgment is prefixed with true.
The Acknowledgement callback can thus be modified to pass in a success boolean value. Making this success value global will make it trivially support multipayload atomic acknowledgments
The text was updated successfully, but these errors were encountered:
I was thinking that we could instead implement a multipass function (rather than multicall) where it still allows multiple functions to be called but doesn't revert the entire tx if one fails. What do you think about this @AdityaSripal ?
Currently any revert in the external contract call of the application callback will revert the entire transaction. This will lead to suboptimal behavior and even errors in our transaction callbacks.
Solution:
Use try/catch for the app callbacks. For OnAck/OnTimeout callbacks, we just ignore the app error and revert the app-level changes while still committting the overall tx. This fixes the relayer issue, and in multipayload we will correctly execute all app callbacks regardless in one app errors.
For OnRecvPacket, we will also wrap in a try/catch. Any error that occurs we will convert into an error ack and add to our acknowledgement list. This correctly reverts the app state if an error is returned while still writing an error ack. This makes writing applications correctly much easier since you do not have to worry about reverting state changes yourself. However, in order to do this with Solidity I needed to create an error acknowledgment in the core handler.
This was difficult to do with the current design since the acknowledgements are completely up to the application. So there was no way that IBC core handler could construct an error ack from an error message since there was no standardized way to describe an error acknowledgment.
This led to prefixing the error coming from the app callback with the boolean
false
to create a standardized error acknowledgment. Thus the success acknowledgment is prefixed withtrue
.The Acknowledgement callback can thus be modified to pass in a success boolean value. Making this success value global will make it trivially support multipayload atomic acknowledgments
The text was updated successfully, but these errors were encountered: