Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions contracts/PolygonTokenBridger.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity ^0.8.0;

import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import "./Lockable.sol";
import "./interfaces/WETH9.sol";

// ERC20s (on polygon) compatible with polygon's bridge have a withdraw method.
interface PolygonIERC20 is IERC20 {
function withdraw(uint256 amount) external;
}

interface MaticToken {
function withdraw(uint256 amount) external payable;
}

// Because Polygon only allows withdrawals from a particular address to go to that same address on mainnet, we need to
// have some sort of contract that can guarantee identical addresses on Polygon and Ethereum.
// Note: this contract is intended to be completely immutable, so it's guaranteed that the contract on each side is
// configured identically as long as it is created via create2. create2 is an alternative creation method that uses
// a different address determination mechanism from normal create.
// Normal create: address = hash(deployer_address, deployer_nonce)
// create2: address = hash(0xFF, sender, salt, bytecode)
// This ultimately allows create2 to generate deterministic addresses that don't depend on the transaction count of the
// sender.
contract PolygonTokenBridger is Lockable {
using SafeERC20 for PolygonIERC20;
using SafeERC20 for IERC20;

MaticToken public constant maticToken = MaticToken(0x0000000000000000000000000000000000001010);
address public immutable destination;
WETH9 public immutable l1Weth;

constructor(address _destination, WETH9 _l1Weth) {
destination = _destination;
l1Weth = _l1Weth;
}

// Polygon side.
function send(
PolygonIERC20 token,
uint256 amount,
bool isMatic
) public nonReentrant {
token.safeTransferFrom(msg.sender, address(this), amount);

// In the wMatic case, this unwraps. For other ERC20s, this is the burn/send action.
token.withdraw(amount);

// This takes the token that was withdrawn and calls withdraw on the "native" ERC20.
if (isMatic) maticToken.withdraw{ value: amount }(amount);
}

// Mainnet side.
function retrieve(IERC20 token) public nonReentrant {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's annoying that we will need a separate bot to execute this method but we could also call it in the executeRelayerRefund and executeSlowRelay methods which are overridable. This would be similar to how OptimismSpokePool works which also has the related problem of receiving ETH over the bridge but needing to have WETH to send to recipients.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The retrieve method would need to be called on the mainnet side, right? executeRelayerRefund/executeSlowRelay are SpokePool methods, which would only happen on polygon, not mainnet right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I thought this TokenBridger contract is meant to assist with tokens sent from HubPool --> Polygon_SpokePool, and back, so couldn't we use the above strategy to at least automate token retrievals on the Polygon side?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need any assistance on the polygon side. I thought the tokens get deposited/sent automatically, no?

token.safeTransfer(destination, token.balanceOf(address(this)));
}

receive() external payable {
// Note: this should only happen on the mainnet side where ETH is sent to the contract directly by the bridge.
if (functionCallStackOriginatesFromOutsideThisContract()) l1Weth.deposit{ value: address(this).balance }();
}
}
99 changes: 99 additions & 0 deletions contracts/Polygon_SpokePool.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
//SPDX-License-Identifier: Unlicense
pragma solidity ^0.8.0;

import "./interfaces/WETH9.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import "./SpokePool.sol";
import "./SpokePoolInterface.sol";
import "./PolygonTokenBridger.sol";

// IFxMessageProcessor represents interface to process messages.
interface IFxMessageProcessor {
function processMessageFromRoot(
uint256 stateId,
address rootMessageSender,
bytes calldata data
) external;
}

/**
* @notice Polygon specific SpokePool.
*/
contract Polygon_SpokePool is SpokePoolInterface, IFxMessageProcessor, SpokePool {
using SafeERC20 for PolygonIERC20;
address public fxChild;
PolygonTokenBridger public polygonTokenBridger;
bool private callValidated = false;

event PolygonTokensBridged(address indexed token, address indexed receiver, uint256 amount);

// Note: validating calls this way ensures that strange calls coming from the fxChild won't be misinterpreted.
// Put differently, just checking that msg.sender == fxChild is not sufficient.
// All calls that have admin priviledges must be fired from within the processMessageFromRoot method that's gone
// through validation where the sender is checked and the root (mainnet) sender is also validated.
// This modifier sets the callValidated variable so this condition can be checked in _requireAdminSender().
modifier validateInternalCalls() {
// This sets a variable indicating that we're now inside a validated call.
// Note: this is used by other methods to ensure that this call has been validated by this method and is not
// spoofed. See
callValidated = true;

_;

// Reset callValidated to false to disallow admin calls after this method exits.
callValidated = false;
}

constructor(
PolygonTokenBridger _polygonTokenBridger,
address _crossDomainAdmin,
address _hubPool,
address _wmaticAddress, // Note: wmatic is used here since it is the token sent via msg.value on polygon.
address _fxChild,
address timerAddress
) SpokePool(_crossDomainAdmin, _hubPool, _wmaticAddress, timerAddress) {
polygonTokenBridger = _polygonTokenBridger;
fxChild = _fxChild;
}

// Note: stateId value isn't used because it isn't relevant for this method. It doesn't care what state sync
// triggered this call.
function processMessageFromRoot(
uint256, /*stateId*/
address rootMessageSender,
bytes calldata data
) public validateInternalCalls {
// Validation logic.
require(msg.sender == fxChild, "Not from fxChild");
require(rootMessageSender == crossDomainAdmin, "Not from mainnet admmin");

// This uses delegatecall to take the information in the message and process it as a function call on this contract.
(bool success, ) = address(this).delegatecall(data);
require(success, "delegatecall failed");
}

/**************************************
* INTERNAL FUNCTIONS *
**************************************/

function _bridgeTokensToHubPool(RelayerRefundLeaf memory relayerRefundLeaf) internal override {
PolygonIERC20(relayerRefundLeaf.l2TokenAddress).safeIncreaseAllowance(
address(polygonTokenBridger),
relayerRefundLeaf.amountToReturn
);

// Note: WETH is WMATIC on matic, so this tells the tokenbridger that this is an unwrappable native token.
polygonTokenBridger.send(
PolygonIERC20(relayerRefundLeaf.l2TokenAddress),
relayerRefundLeaf.amountToReturn,
address(weth) == relayerRefundLeaf.l2TokenAddress
);

emit PolygonTokensBridged(relayerRefundLeaf.l2TokenAddress, address(this), relayerRefundLeaf.amountToReturn);
}

function _requireAdminSender() internal view override {
require(callValidated, "Must call processMessageFromRoot");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a clever way to check that calls are only made via the fx tunnel. But could you not replace this call with a require that the msg.sender is the fxChild and remove the callValidated Boolean variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! This was something that I considered. The particular case that I'm worried about (and maybe I shouldn't be) is that the polygon fxChild contract could make other calls into contracts (now or in the future) on behalf of mainnet callers. If 1) those methods happen to be the same name/args as one of our admin methods or 2) (more likely) there's a way to partially or fully customize the selector used (hash of method name + arg types), someone could make a malicious call that comes from that contract but hasn't had its sender validated.

The types of attacks I'm worried about are ones in the style of the poly network hack, where someone is able to use user-inputs to force a contract to call another contract in a way that it wasn't intended to. https://twitter.com/kelvinfichter/status/1425217065690992641?s=20&t=OL5ecSja3ii-LktomykOOg.

I doubt this is possible on polygon today, but their contracts are upgradable and I just figure it's better to be extra careful.

Does that make sense? Thoughts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's a reasonable concern and one worth documenting! Two comments:

  • I think you should add the callValidated flipping on/ff as a modifier for readability
  • You should add a comment explaining why checking msg.sender == fxChild is not sufficient because of the reasons you explained above

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}
}
2 changes: 1 addition & 1 deletion contracts/SpokePool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol";
import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";

import "@uma/core/contracts/common/implementation/Testable.sol";
import "@uma/core/contracts/common/implementation/Lockable.sol";
import "@uma/core/contracts/common/implementation/MultiCaller.sol";
import "./Lockable.sol";
import "./MerkleLib.sol";
import "./SpokePoolInterface.sol";

Expand Down
4 changes: 3 additions & 1 deletion contracts/chain-adapters/Optimism_Adapter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@ import "@eth-optimism/contracts/L1/messaging/IL1StandardBridge.sol";
import "@uma/core/contracts/common/implementation/Lockable.sol";

import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

/**
* @notice Sends cross chain messages Optimism L2 network.
* @dev This contract's owner should be set to the some multisig or admin contract. The Owner can simply set the L2 gas
* and the HubPool. The HubPool is the only contract that can relay tokens and messages over the bridge.
*/
contract Optimism_Adapter is Base_Adapter, CrossDomainEnabled, Lockable {
using SafeERC20 for IERC20;
uint32 public l2GasLimit = 5_000_000;

WETH9 public l1Weth;
Expand Down Expand Up @@ -57,7 +59,7 @@ contract Optimism_Adapter is Base_Adapter, CrossDomainEnabled, Lockable {
l1Weth.withdraw(amount);
l1StandardBridge.depositETHTo{ value: amount }(to, l2GasLimit, "");
} else {
IERC20(l1Token).approve(address(l1StandardBridge), amount);
IERC20(l1Token).safeIncreaseAllowance(address(l1StandardBridge), amount);
l1StandardBridge.depositERC20To(l1Token, l2Token, to, amount, l2GasLimit, "");
}
emit TokensRelayed(l1Token, l2Token, amount, to);
Expand Down
72 changes: 72 additions & 0 deletions contracts/chain-adapters/Polygon_Adapter.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity ^0.8.0;

import "./Base_Adapter.sol";
import "../interfaces/AdapterInterface.sol";
import "../interfaces/WETH9.sol";

import "@eth-optimism/contracts/libraries/bridge/CrossDomainEnabled.sol";
import "@eth-optimism/contracts/L1/messaging/IL1StandardBridge.sol";
import "../Lockable.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

interface IRootChainManager {
function depositEtherFor(address user) external payable;

function depositFor(
address user,
address rootToken,
bytes calldata depositData
) external;
}

interface IFxStateSender {
function sendMessageToChild(address _receiver, bytes calldata _data) external;
}

/**
* @notice Sends cross chain messages Polygon L2 network.
*/
contract Polygon_Adapter is Base_Adapter, Lockable {
using SafeERC20 for IERC20;
IRootChainManager public rootChainManager;
IFxStateSender public fxStateSender;
WETH9 public l1Weth;

constructor(
address _hubPool,
IRootChainManager _rootChainManager,
IFxStateSender _fxStateSender,
WETH9 _l1Weth
) Base_Adapter(_hubPool) {
rootChainManager = _rootChainManager;
fxStateSender = _fxStateSender;
l1Weth = _l1Weth;
}

function relayMessage(address target, bytes memory message) external payable override nonReentrant onlyHubPool {
fxStateSender.sendMessageToChild(target, message);
emit MessageRelayed(target, message);
}

function relayTokens(
address l1Token,
address l2Token,
uint256 amount,
address to
) external payable override nonReentrant onlyHubPool {
// If the l1Token is weth then unwrap it to ETH then send the ETH to the standard bridge.
if (l1Token == address(l1Weth)) {
l1Weth.withdraw(amount);
rootChainManager.depositEtherFor{ value: amount }(to);
} else {
IERC20(l1Token).safeIncreaseAllowance(address(rootChainManager), amount);
rootChainManager.depositFor(to, l1Token, abi.encode(amount));
}
emit TokensRelayed(l1Token, l2Token, amount, to);
}

// Added to enable the Polygon_Adapter to receive ETH. used when unwrapping WETH.
receive() external payable {}
}
13 changes: 13 additions & 0 deletions contracts/test/PolygonERC20Test.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity ^0.8.0;

import "@uma/core/contracts/common/implementation/ExpandedERC20.sol";
import "../PolygonTokenBridger.sol";

contract PolygonERC20Test is ExpandedERC20, PolygonIERC20 {
constructor() ExpandedERC20("Polygon Test", "POLY_TEST", 18) {}

function withdraw(uint256 amount) public {
_burn(msg.sender, amount);
}
}
16 changes: 16 additions & 0 deletions contracts/test/PolygonMocks.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity ^0.8.0;

contract RootChainManagerMock {
function depositEtherFor(address user) external payable {}

function depositFor(
address user,
address rootToken,
bytes calldata depositData
) external {}
}

contract FxStateSenderMock {
function sendMessageToChild(address _receiver, bytes calldata _data) external {}
}
Loading