extras/axelar-token: improve error handling and add recovery paths. (#…

…814)

extras/axelar-token/.env.example

@@ -2,38 +2,53 @@
 ## To change the network to use as origin and destination, modify these following environment variables.
 ## Other environment variables are prefixed with the network name, followed by two underscores.
 ## For example, for the Polygon Mumbai network, the private key environment variable is POLYGON_MUMBAI__PRIVATE_KEY
-ORIGIN_NETWORK=POLYGON_MUMBAI  # Specifies the origin network as Polygon Mumbai
-DEST_NETWORK=FILECOIN_CALIBRATION  # Specifies the destination network as Filecoin Calibration
+# Specifies the origin network as Polygon Mumbai
+ORIGIN_NETWORK=POLYGON_MUMBAI
+# Specifies the destination network as Filecoin Calibration
+DEST_NETWORK=FILECOIN_CALIBRATION
 
 ###
 ### Polygon Mumbai
 #########################
 
-POLYGON_MUMBAI__PRIVATE_KEY=  # Private key for the Polygon Mumbai network
-POLYGON_MUMBAI__RPC_URL=https://polygon-mumbai.gateway.tenderly.co  # RPC URL for the Polygon Mumbai network
+# Private key for the Polygon Mumbai network
+POLYGON_MUMBAI__PRIVATE_KEY=
+# RPC URL for the Polygon Mumbai network
+POLYGON_MUMBAI__RPC_URL=https://polygon-mumbai.gateway.tenderly.co
 
 # Axelar config; see https://docs.axelar.dev/resources/contract-addresses/testnet
-POLYGON_MUMBAI__AXELAR_ITS_ADDRESS=0xB5FB4BE02232B1bBA4dC8f81dc24C26980dE9e3C  # Axelar ITS address for the Polygon Mumbai network
-POLYGON_MUMBAI__AXELAR_CHAIN_NAME=Polygon  # Axelar chain name for the Polygon Mumbai network
+# Axelar ITS address for the Polygon Mumbai network
+POLYGON_MUMBAI__AXELAR_ITS_ADDRESS=0xB5FB4BE02232B1bBA4dC8f81dc24C26980dE9e3C
+# Axelar chain name for the Polygon Mumbai network
+POLYGON_MUMBAI__AXELAR_CHAIN_NAME=Polygon
 
 ###
 ### Filecoin Calibration
 #########################
 
-FILECOIN_CALIBRATION__PRIVATE_KEY=  # Private key for the Filecoin Calibration network
-FILECOIN_CALIBRATION__RPC_URL=https://api.calibration.node.glif.io/rpc/v1  # RPC URL for the Filecoin Calibration network
+# Private key for the Filecoin Calibration network
+FILECOIN_CALIBRATION__PRIVATE_KEY=
+# RPC URL for the Filecoin Calibration network
+FILECOIN_CALIBRATION__RPC_URL=https://api.calibration.node.glif.io/rpc/v1
 
 # Axelar config; see https://docs.axelar.dev/resources/contract-addresses/testnet
-FILECOIN_CALIBRATION__AXELAR_ITS_ADDRESS=0xB5FB4BE02232B1bBA4dC8f81dc24C26980dE9e3C  # Axelar ITS address for the Filecoin Calibration network
-FILECOIN_CALIBRATION__AXELAR_CHAIN_NAME=filecoin-2  # Axelar chain name for the Filecoin Calibration network
+# Axelar ITS address for the Filecoin Calibration network
+FILECOIN_CALIBRATION__AXELAR_ITS_ADDRESS=0xB5FB4BE02232B1bBA4dC8f81dc24C26980dE9e3C
+# Axelar chain name for the Filecoin Calibration network
+FILECOIN_CALIBRATION__AXELAR_CHAIN_NAME=filecoin-2
 
 # IPC
-FILECOIN_CALIBRATION__IPC_GATEWAY_ADDRESS=0xdd0A8b2f247b8444AF716b14250939B57cdBe1E8  # IPC gateway address for the Filecoin Calibration network
+# IPC gateway address for the Filecoin Calibration network
+FILECOIN_CALIBRATION__IPC_GATEWAY_ADDRESS=0xdd0A8b2f247b8444AF716b14250939B57cdBe1E8
+# The admin address of the IpcTokenHandler, which is authorized to operate the treasury to recover funds.
+FILECOIN_CALIBRATION__HANDLER_ADMIN_ADDRESS=0x
 
 ###
 ### Axelar token addresses
 ################################
 
 # https://testnet.interchain.axelar.dev/polygon/0x0cc94E0b1056Ab7125DD820b406C0A4581CCec91
-POLYGON_MUMBAI__ORIGIN_TOKEN_ADDRESS=0x0cc94E0b1056Ab7125DD820b406C0A4581CCec91  # Origin token address for the Polygon Mumbai network
-FILECOIN_CALIBRATION__REMOTE_TOKEN_ADDRESS=0x0cc94E0b1056Ab7125DD820b406C0A4581CCec91  # Remote token address for the Filecoin Calibration network
+# Origin token address for the Polygon Mumbai network
+POLYGON_MUMBAI__ORIGIN_TOKEN_ADDRESS=0x0cc94E0b1056Ab7125DD820b406C0A4581CCec91
+# Remote token address for the Filecoin Calibration network
+FILECOIN_CALIBRATION__REMOTE_TOKEN_ADDRESS=0x0cc94E0b1056Ab7125DD820b406C0A4581CCec91

extras/axelar-token/script/Deploy.s.sol

@@ -17,7 +17,8 @@ contract Deploy is Script {
         vm.startBroadcast(privateKey);
         IpcTokenHandler handler = new IpcTokenHandler({
             axelarIts: vm.envAddress(string.concat(network, "__AXELAR_ITS_ADDRESS")),
-            ipcGateway: vm.envAddress(string.concat(network, "__IPC_GATEWAY_ADDRESS"))
+            ipcGateway: vm.envAddress(string.concat(network, "__IPC_GATEWAY_ADDRESS")),
+            admin: vm.envAddress(string.concat(network, "__HANDLER_ADMIN_ADDRESS"))
         });
         vm.stopBroadcast();
 

extras/axelar-token/src/IpcTokenHandler.sol

@@ -3,6 +3,7 @@ pragma solidity ^0.8.20;
 
 import { InterchainTokenExecutable } from '@axelar-network/interchain-token-service/executable/InterchainTokenExecutable.sol';
 import { IERC20 } from "openzeppelin-contracts/interfaces/IERC20.sol";
+import { Ownable } from "openzeppelin-contracts/access/Ownable.sol";
 import { SubnetID, SupplySource, SupplyKind } from "@ipc/src/structs/Subnet.sol";
 import { FvmAddress } from "@ipc/src/structs/FvmAddress.sol";
 import { IpcHandler } from "@ipc/sdk/IpcContract.sol";
@@ -23,21 +24,20 @@ interface SubnetActor {
 //         IpcTokenSender via the Axelar ITS, receiving some token value to deposit into an IPC subnet (specified in the
 //         incoming message). The IpcTokenHandler handles deposit failures by crediting the value back to the original
 //         beneficiary, and making it available from them to withdraw() on the rootnet.
-contract IpcTokenHandler is InterchainTokenExecutable, IpcHandler {
+contract IpcTokenHandler is InterchainTokenExecutable, IpcHandler, Ownable {
     using FvmAddressHelper for address;
     using FvmAddressHelper for FvmAddress;
     using SubnetIDHelper for SubnetID;
     using SafeERC20 for IERC20;
 
     error NothingToWithdraw();
 
-    TokenFundedGateway public _ipcGateway;
-    mapping(address beneficiary => mapping(address token => uint256 value)) private _claims;
-
     event SubnetFunded(SubnetID indexed subnet, address indexed recipient, uint256 value);
     event FundingFailed(SubnetID indexed subnet, address indexed recipient, uint256 value);
 
-    constructor(address axelarIts, address ipcGateway) InterchainTokenExecutable(axelarIts) {
+    TokenFundedGateway public _ipcGateway;
+
+    constructor(address axelarIts, address ipcGateway, address admin) InterchainTokenExecutable(axelarIts) Ownable(admin) {
         _ipcGateway = TokenFundedGateway(ipcGateway);
     }
 
@@ -52,18 +52,33 @@ contract IpcTokenHandler is InterchainTokenExecutable, IpcHandler {
         address tokenAddr,
         uint256 amount
     ) internal override {
-        (SubnetID memory subnet, address recipient) = abi.decode(data, (SubnetID, address));
-
         IERC20 token = IERC20(tokenAddr);
         require(token.balanceOf(address(this)) >= amount, "insufficient balance");
 
         // Authorize the IPC gateway to spend these tokens on our behalf.
-        token.approve(address(_ipcGateway), amount);
+        token.safeIncreaseAllowance(address(_ipcGateway), amount);
+
+        // Try to decode the payload. Note: Solidity does not support try/catch for abi.decode (or tryDecode), so
+        // this may fail if there's a bug in the sender (in which case funds can be retrieved through the admin path).
+        (SubnetID memory subnet, address recipient) = abi.decode(data, (SubnetID, address));
+
+        (bool success, ) = address(_ipcGateway).call(
+            abi.encodeWithSelector(TokenFundedGateway.fundWithToken.selector, subnet, recipient.from(), amount)
+        );
 
-        // Fund the designated subnet via the IPC gateway.
-        _ipcGateway.fundWithToken(subnet, recipient.from(), amount);
+        if (!success) {
+            // Restore the original allowance.
+            token.safeDecreaseAllowance(address(_ipcGateway), amount);
+
+            // Increase the allowance of the admin address so they can retrieve these otherwise lost tokens.
+            token.safeIncreaseAllowance(owner(), amount);
+
+            // Emit a FundingFailed event.
+            emit FundingFailed(subnet, recipient, amount);
+
+            return;
+        }
 
-        // Emit an event.
         emit SubnetFunded(subnet, recipient, amount);
     }
 
@@ -78,38 +93,27 @@ contract IpcTokenHandler is InterchainTokenExecutable, IpcHandler {
 
         ResultMsg memory result = abi.decode(envelope.message, (ResultMsg));
         if (result.outcome != OutcomeType.Ok) {
-            // Note: IPC only supports deploying subnets via our blessed registry, so we can trust the code behind
-            // the subnet actor.
+            // Verify that the subnet is indeed an ERC20 subnet.
             SupplySource memory supplySource = SubnetActor(envelope.from.subnetId.getAddress()).supplySource();
             require(supplySource.kind == SupplyKind.ERC20, "expected ERC20 supply source");
 
+            // Increase the allowance of the admin address so they can retrieve these otherwise lost tokens.
+            IERC20(supplySource.tokenAddress).safeIncreaseAllowance(owner(), envelope.value);
+
             // Results will carry the original beneficiary in the 'from' address.
             address beneficiary = envelope.from.rawAddress.extractEvmAddress();
 
-            // We credit the token funds to the beneficiary. The beneficiary will have to call withdraw() to pull the
-            // funds out on this network.
-            _claims[beneficiary][supplySource.tokenAddress] += envelope.value;
-
             // Emit an event.
             emit FundingFailed(envelope.from.subnetId, beneficiary, envelope.value);
         }
 
         return bytes("");
     }
 
-    // @notice Withdraws all available balance for the specified token for the sender.
-    function withdraw(address token) external {
-        uint256 available = _claims[msg.sender][token];
-        if (available == 0) {
-            revert NothingToWithdraw();
-        }
-
-        delete _claims[msg.sender][token];
-        IERC20(token).safeTransfer(msg.sender, available);
+    // @notice The ultimate backstop in case the error-handling logic itself failed unexpectedly and we failed to
+    //         increase the recovery allowances of the admin address.
+    function adminTokenIncreaseAllowance(address token, uint256 amount) external onlyOwner {
+        IERC20(token).safeIncreaseAllowance(owner(), amount);
     }
 
-    // @notice Queries the claim of a beneficiary over a particular token.
-    function getClaimFor(address beneficiary, address token) external view returns (uint256) {
-        return _claims[beneficiary][token];
-    }
 }

extras/axelar-token/test/TestHandler.sol

@@ -12,19 +12,21 @@ contract TestHandler is Test {
     function test_handler_Ok() public {
         address axelarIts = vm.addr(1);
         address ipcGateway = vm.addr(2);
+        address owner = vm.addr(3);
         DummyERC20 token = new DummyERC20("Test token", "TST", 10000);
 
         IpcTokenHandler handler = new IpcTokenHandler({
             axelarIts: axelarIts,
-            ipcGateway: ipcGateway
+            ipcGateway: ipcGateway,
+            admin: owner
         });
 
         address[] memory route = new address[](1);
         route[0] = 0x2a3eF0F414c626e51AFA2F29f3F7Be7a45C6DB09;
+        SubnetID memory subnet = SubnetID({ root: 314159, route: route });
 
         address recipient = 0x6B505cdCCCA34aE8eea5D382aBaD40d2AfEa74ad;
 
-        SubnetID memory subnet = SubnetID({ root: 314159, route: route });
         bytes memory params = abi.encode(subnet, recipient);
 
         token.transfer(address(handler), 1);
@@ -36,10 +38,91 @@ contract TestHandler is Test {
             abi.encode("")
         );
         handler.executeWithInterchainToken(bytes32(""), "", "", params, bytes32(""), address(token), 1);
+
+        // the allowance of the gateway is still 1, because the call to fundWithToken was mocked and did not actually expend the allowance
+        // this is not what would happen in reality, but the assert gives us extra insight
+        require(token.allowance(address(handler), ipcGateway) == 1);
     }
 
-    // TODO test_handler_err_withdrawal (also test getClaims)
+    function test_handler_failGateway() public {
+        address axelarIts = vm.addr(1);
+        address ipcGateway = vm.addr(2);
+        address owner = vm.addr(3);
+        DummyERC20 token = new DummyERC20("Test token", "TST", 10000);
+
+        IpcTokenHandler handler = new IpcTokenHandler({
+            axelarIts: axelarIts,
+            ipcGateway: ipcGateway,
+            admin: owner
+        });
 
-    // TODO test_handler_err_deposit (e.g. sending to a non-ERC20 subnet)
+        address[] memory route = new address[](1);
+        route[0] = 0x2a3eF0F414c626e51AFA2F29f3F7Be7a45C6DB09;
+        SubnetID memory subnet = SubnetID({ root: 314159, route: route });
+
+        address recipient = 0x6B505cdCCCA34aE8eea5D382aBaD40d2AfEa74ad;
+
+        bytes memory params = abi.encode(subnet, recipient);
+
+        token.transfer(address(handler), 1);
+        vm.startPrank(axelarIts);
+
+        vm.expectEmit();
+        emit IERC20.Approval(address(handler), address(ipcGateway), 1);
+        emit IERC20.Approval(address(handler), address(ipcGateway), 0);
+        emit IERC20.Approval(address(handler), address(owner), 1);
+        emit IpcTokenHandler.FundingFailed(subnet, recipient, 1);
+
+        vm.mockCallRevert(
+            address(ipcGateway),
+            abi.encodeWithSelector(TokenFundedGateway.fundWithToken.selector, subnet, recipient.from(), 1),
+            abi.encode("ERROR")
+        );
+        handler.executeWithInterchainToken(bytes32(""), "", "", params, bytes32(""), address(token), 1);
+
+        // the allowance was accrued to the owner
+        require(token.allowance(address(handler), ipcGateway) == 0);
+        require(token.allowance(address(handler), owner) == 1);
+    }
+
+    function test_handler_fail_unexpected() public {
+        address axelarIts = vm.addr(1);
+        address ipcGateway = vm.addr(2);
+        address owner = vm.addr(3);
+        DummyERC20 token = new DummyERC20("Test token", "TST", 10000);
+
+        IpcTokenHandler handler = new IpcTokenHandler({
+            axelarIts: axelarIts,
+            ipcGateway: ipcGateway,
+            admin: owner
+        });
+
+        // garbage
+        bytes memory params = abi.encode(1);
+
+        token.transfer(address(handler), 4200);
+        vm.startPrank(axelarIts);
+
+        // will revert due to garbage.
+        vm.expectRevert();
+        handler.executeWithInterchainToken(bytes32(""), "", "", params, bytes32(""), address(token), 4200);
+
+        // let's ensure we can recover the tokens
+        require(token.allowance(address(handler), owner) == 0);
+
+        // this should revert when called by the previous pranked account (non-owner).
+        vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, axelarIts));
+        handler.adminTokenIncreaseAllowance(address(token), 4200);
+
+        // now act like the owner.
+        vm.startPrank(owner);
+
+        handler.adminTokenIncreaseAllowance(address(token), 4200);
+        require(token.allowance(address(handler), owner) == 4200);
+
+        token.transferFrom(address(handler), owner, 4200);
+        require(token.allowance(address(handler), owner) == 0);
+        require(token.balanceOf(owner) == 4200);
+    }
 
 }