Restricting Access to Solidity Callback Function

Question

I have a solidity contract that executes arbitrage trades with flash loans. In order for the flash loan pool to call the contract callback function after transfering the funds to my contract, that function has to be externally callable. Here is the function that the loan pools calls:

function _flashLoanCallBack(address sender, uint256, uint256, bytes calldata data) internal {
    (address flashLoanPool, address token1, address token2, address token3, address token4, address _router1, address _router2, address _router3, uint256 amount) = abi.decode(data, (address, address, address, address, address, address, address, address, uint256));
    require(sender == address(this) && msg.sender == flashLoanPool, "HANDLE_FLASH_NENIED");
    if (token4 == address(0x000000000000000000000000000000000000000F)) {
        if (token3 == address(0x000000000000000000000000000000000000000F)) {
            this.dualDexTrade(_router1, _router2, token1, token2, amount);}
        else {
            this.triDexTrade(_router1, _router2, _router3, token1, token2, token3, amount);
            }
    }else {
        this._instaTrade(_router1, token1, token2, token3, token4, amount);
}
    IERC20(token1).transfer(flashLoanPool, amount);
}

When the pool triggers that function, it than calls one of the trade functions that executes the trade. In order to get this to work I have had to keep the visibility on that function external.

function dualDexTrade(address _router1, address _router2, address _token1, address _token2, uint256 _amount) external {
    uint startBalance = IERC20(_token1).balanceOf(address(this));
    uint token2InitialBalance = IERC20(_token2).balanceOf(address(this));
    swap(_router1, _token1, _token2, _amount);
    uint token2Balance = IERC20(_token2).balanceOf(address(this));
    uint tradeableAmount = token2Balance - token2InitialBalance;
    swap(_router2, _token2, _token1, tradeableAmount);
    uint endBalance = IERC20(_token1).balanceOf(address(this));
    require(endBalance > startBalance, "Trade Reverted, No Profit Made");
}

This function of course cannot be restricted to onlyOwner, and I do not want to hardcode the pool addresses or store them on chain if I can help it. I tried setting the dualDexTrade function visibility to internal, but that does not work (all of the trades revert), although I am not sure why, as it is being called from within my contract.

So the question is: How can I secure these functions without breaking it, avoiding having to use hardcoded loan pool address require statements? Also, feel free to give me pointers on how to improve this code, I haven't been writing Solidity for very long. Thank you.

Answer 1

It turns out that the solution in this case was to restrict access via tx.origin.

msg.sender == caller of current call tx.origin == caller that originated the transaction

In this case, I am initiating the transaction from EOA. My contract then calls the flash loan pool (msg.sender is contract address for this call), which then calls my contract (msg.sender is the loan pool). Throughout all of these calls however, the tx.origin will remain my externally owned address, so this is the most secure way to limit access.

The reason that I was not able to change the function visibility to internal from external is because I was calling that function via this, which will call my contract externally, and thus interal methods are not available.

I figured all of this out by simply playing around in remix. In case someone else comes across this and would like to understand how it works, try this code in remix which is simulating the arbitrage contract:

// SPDX-License-Identifier:MIT
pragma solidity ^0.8.0;

contract Arbitrage {
address owner;

address public lastMsgSender;
address public lastTxOrigin;
bool public OKAY = false;

constructor() {
    owner = msg.sender;
}

modifier onlyOwner{
    require(owner == msg.sender || owner == tx.origin);
    _;
}

function execute() internal {
    OKAY = true;

}

function receiveCall() external  {
    lastMsgSender = msg.sender;
    lastTxOrigin = tx.origin;
    execute();
}

function triggerCall(address target) external onlyOwner returns(bool) {
    // Call this function with address of LoanPool
    (bool success, ) = target.call(abi.encodeWithSignature("flashLoan()"));
    return success;
  }
}

And this representing the loan pool:

contract LoanPool {
function flashLoan() external returns(bool){
    (bool success, ) = msg.sender.call(abi.encodeWithSignature("receiveCall()"));
    return success;
  }
}