-
Notifications
You must be signed in to change notification settings - Fork 695
EVM Contract Callbacks #8335
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
Comments
After some initial investigation, I propose the following: The callbacks middleware will be adjusted to include an additional field in the // CallbackData is the callback data parsed from the packet.
type CallbackData struct {
// CallbackAddress is the address of the callback actor.
CallbackAddress string
// ExecutionGasLimit is the gas limit which will be used for the callback execution.
ExecutionGasLimit uint64
// SenderAddress is the sender of the packet. This is passed to the contract keeper
// to verify that the packet sender is the same as the callback address if desired.
// This address is empty during destination callback execution.
// This address may be empty if the sender is unknown or undefined.
SenderAddress string
// CommitGasLimit is the gas needed to commit the callback even if the callback
// execution fails due to out of gas.
// This parameter is only used in event emissions, or logging.
CommitGasLimit uint64
// ApplicationVersion is the base application version.
ApplicationVersion string
+ // Calldata is the custom calldata for the actor
+ Calldata []byte
} This will be hex-encoded in the memo. When it is specified, the contract actor is called directly with the calldata provided and no additional information. This allows pre-existing contracts to be deployed and called through EVM-callbacks by just specifying the ABI-packed data it expects. In EVM-callbacks contract keeper we will pass this calldata to this function: https://github.com/cosmos/evm/blob/a828f95ffc3e8c636a07e657c380eaf26f2123e0/x/vm/keeper/call_evm.go#L46 // CallEVMWithData performs a smart contract method call using contract data.
func (k Keeper) CallEVMWithData(
ctx sdk.Context,
from common.Address,
contract *common.Address,
data []byte,
commit bool,
) I believe this will provide the cleanest and long-term solution for EVM callbacks of the two options.
The exact distribution of work to implement the contract keeper can be decided as we wish so long as the repo locations are agreed upon. The alternative I briefly entertained was making no changes to ibc-go. This would require a new field in the memo such that it looked like: {
"callback-actor": addr,
"gas-limit": 5000,
},
{
"evm-calldata": data,
} as opposed to the current proposal: {
"callback-actor": addr,
"gas-limit": 5000,
"calldata": data,
} The latter is much more user-friendly and understandable and is what the ideal solution would look like. Given the minimal changes required of ibc-go, I think we should go in the direction I describe above. |
#8353 outlines the minimal changes needed to ibc-go to support this feature |
I think this proposal makes sense. It is simple and solves the problem. One question, when you say:
How do you mean they specify the ABI-packed data it expects? As in documentation or something else? |
@gjermundgaraba the bytes passed in would need to be packed using an ABI offchain, instead of supplying the ABI itself. See here for the latter logic: https://github.com/cosmos/evm/blob/db7fef84ca727c80b646d31e39a30faf441233a3/x/vm/keeper/call_evm.go#L22-L43 The packing would instead be done offchain and the bytes expected in the callbacks would be pre-packed. Overall, the proposal LGTM. Simple, and I like that it doesn't require contracts to write specific entrypoints for contracts. |
What does the flow look like for calls that required erc20 approvals? |
In this case, we expect that the receiver of the ICS20 tokens are the same as the contract callback actor. Thus, it can then approve any spends on its own behalf even if it then needs to call an external contract that moves funds. It can also then send the IBC funds to a final intended receiver. Does this model work with the existing skip contracts? If not, I would need some context on how these are currently working |
The callbacks middleware was designed to support arbitrary VMs, thus it should be able to support callbacks to EVM contracts by implementing a custom contract keeper that satisfies the interface expected by the callbacks middleware. Many of the usecases for EVM require that we can support contracts that are already deployed and thus do not need to change.
The callbacks middleware expects a field in the memo with the folllowing data:
Here there is no place to pass in the data needed for the contract directly. Thus, callbacks was originally designed to have its own unique endpoint on the contract that it could call and pass in the packet data.
Since this is no longer the desired case, we could add in a field that allows us to pass in the custom data to the contract. Then only this data gets passed into the
EVM calldata
. This will allow us to call any pre-deployed contract function with arbitrary data even if the contract is not aware at all about callbacks.So one option is to make changes to the callbacks middleware to explicitly do this.
Another option is to allow the custom contract keeper to parse this information from the packet data. The contract keeper can still receive the exact same arguments, but it does not need to send the packet data to the contract. Instead it can parse the packet data to find the contract calldata and send it to the contract. The benefit of this approach is that it requires no changes to the ibc-go callbacks middleware, however it does require another top-level field in the json memo that can provide this additional information.
The text was updated successfully, but these errors were encountered: