Execute multiple contract calls atomically with optional sweep-on-failure#1702
Execute multiple contract calls atomically with optional sweep-on-failure#1702
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1702 +/- ##
==========================================
+ Coverage 81.04% 81.14% +0.09%
==========================================
Files 22 22
Lines 997 1034 +37
Branches 186 195 +9
==========================================
+ Hits 808 839 +31
- Misses 172 177 +5
- Partials 17 18 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
yrong
changed the title
yrong
changed the title
| external | ||
| onlySelf | ||
| { | ||
| CallContractsParams memory params = abi.decode(payload, (CallContractsParams)); |
There was a problem hiding this comment.
Params are decoded here for a second time, can probably send CallContractsParams to trySweepOnFailure.
| CallContractParams[] memory params = new CallContractParams[](1); | ||
| params[0] = | ||
| CallContractParams({target: address(0xdead), data: "", value: uint256(0)}); | ||
| bytes memory payload = abi.encode(params); |
There was a problem hiding this comment.
| bytes memory payload = abi.encode(params); | |
| CallContractsParams memory p = CallContractsParams({ | |
| calls: params, | |
| sweepRecipient: address(0), | |
| tokensToSweep: new address[](0) | |
| }); | |
| bytes memory payload = abi.encode(p); |
Since it should be a CallContractsParams struct, not CallContractParams[] array.
| } | ||
|
|
||
| // Sweep remaining assets when specified | ||
| function sweep(address recipient, address[] calldata tokens) external { |
There was a problem hiding this comment.
Can unrelated assets be swept unintentionally here?
There was a problem hiding this comment.
Typically, the sweep token list is constructed by the SDK and kept consistent with the transfer token list.
By the way, the sweep flow is optional and can be skipped by providing an empty address.
There was a problem hiding this comment.
So I like this PR in theory. But I would like to build this at a higher level. So here we are mimicking the Asset Claimer behaviour by using (maybe abusing) Transact and the fact that we can implement anything via arbitrary contract call, rather than sticking close to the XCM functionality and build it into our protocol directly.
For instance we should probably stick closer to the XCM spec, and by default always sweep assets on any failure. By default we should sweep assets to the locations Agent, however if SetHints{Claimer} is set then then we use that account. The asset list should be filled in by the on-chain code from the XCM ground truth instead of expecting the sdk to build it correctly offchain. Also if a user has an agent, JIT create the Agent if it does not exist. Jit creating an agent and paying more gas, but having funds recoverable is probably better than failing and having funds trapped in limbo.
If a user didnt specify any claimer and the funds got sent to the agent, we could use ClaimAsset instruction to get them back for instance.
3 participants
Summary
Adds optional sweep support to the CallContracts command so that when sub-calls fail, configured tokens (including ETH) can be swept to a recipient. This recovers assets that would otherwise be stuck in the agent when a call reverts.
Behavior