Adding an option for apps to 'forecall' callContractWithToken to hasten resolution of calls. #102
Conversation
Co-authored-by: Sammy Liu <sammy.liu@axelar.network>
Foivos
changed the title
|
I need more information about what is this PR doing and what are we trying to achieve before I can review it. |
test/Util.js
Outdated
|
|
||
| const UtilTest = require('../build/UtilTest.json'); | ||
|
|
||
| const { it } = require('mocha'); |
There was a problem hiding this comment.
| const { it } = require('mocha'); |
| import { IAxelarGateway } from './IAxelarGateway.sol'; | ||
| import { IERC20 } from './IERC20.sol'; | ||
|
|
||
| abstract contract IAxelarExecutableForetellable { |
There was a problem hiding this comment.
| abstract contract IAxelarExecutableForetellable { | |
| abstract contract AxelarExecutableForetellable { |
I is for interface and this isn't but an abstract contract.
There was a problem hiding this comment.
@sammy1991106 we already used that pattern for the IAxelarExecutable. It has some logic in it and also is an abstract contract. Do you think it's worth renaming it? Also should it be moved out of interfaces folder?
There was a problem hiding this comment.
ok nvm then if it's already done elsewhere.
| error NotAuthorizedToForetell(); | ||
|
|
||
| IAxelarGateway public gateway; | ||
| mapping(string => mapping(string => mapping(bytes => mapping(string => mapping(uint256 => address))))) |
There was a problem hiding this comment.
| mapping(string => mapping(string => mapping(bytes => mapping(string => mapping(uint256 => address))))) | |
| mapping(bytes32 => address) public foretellers; |
and key becomes keccak256(sourceChain, sourceAddress, payload, tokenSymbol, amount). Fewer hashing happening and therefore cheaper gas cost, no?
| string memory tokenSymbolA, | ||
| uint256 amount | ||
| ) internal override { | ||
| (string memory tokenSymbolB, string memory recipient) = abi.decode(payload, (string, string)); |
There was a problem hiding this comment.
| (string memory tokenSymbolB, string memory recipient) = abi.decode(payload, (string, string)); | |
| (bytes32 nonce, string memory tokenSymbolB, string memory recipient) = abi.decode(payload, (string, string)); |
Without a nonce, only one of the multiple swapping attempts with the exact same parameters can be foretold until executed.
| _execute(sourceChain, sourceAddress, payload); | ||
| } | ||
|
|
||
| function foretell( |
There was a problem hiding this comment.
I don't see why foretell can only work with executeWithToken but not execute?
|
Please rebase your branch onto the latest main 💯 |
| } else { | ||
| _execute(sourceChain, sourceAddress, payload); | ||
| } | ||
| _execute(sourceChain, sourceAddress, payload); |
There was a problem hiding this comment.
Is this _execute duplicated here?
|
|
||
| abstract contract IAxelarExecutableForetellable { | ||
| error NotApprovedByGateway(); | ||
| error AlreadyFortold(); |
| address foreteller | ||
| ) external { | ||
| address token = gateway.tokenAddresses(tokenSymbol); | ||
| IERC20(token).transferFrom(msg.sender, address(this), amount); |
There was a problem hiding this comment.
We need to use safe transfer here
Example
| if (foreteller != address(0)) { | ||
| foretellers[keccak256(abi.encode(sourceChain, sourceAddress, payload, tokenSymbol, amount))] = address(0); | ||
| address token = gateway.tokenAddresses(tokenSymbol); | ||
| IERC20(token).transfer(foreteller, amount); |
re1ro
changed the title
…estinationChainSwapForecallable.sol
Add a different version of the executable that can have executeWithToken front-run by the application (and have the tokens provided to it) to hasten execution.