-
Notifications
You must be signed in to change notification settings - Fork 75
fix: polygon testnet fixes #128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
330de58
c65505a
ceab201
bbb326c
9dcdd7f
8449857
2ca5759
0a7d597
3a10bb9
9b0098d
5058922
32f06d5
6f0d0e5
baaa78e
811ac20
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,10 @@ | ||
| // SPDX-License-Identifier: AGPL-3.0-only | ||
| pragma solidity ^0.8.0; | ||
|
|
||
| import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; | ||
|
|
||
| contract RootChainManagerMock { | ||
| function depositEtherFor(address user) external payable {} // solhint-disable-line no-empty-blocks | ||
| function depositEtherFor(address user) external payable {} | ||
|
|
||
| function depositFor( | ||
| address user, | ||
|
|
@@ -15,3 +17,29 @@ contract FxStateSenderMock { | |
| // solhint-disable-next-line no-empty-blocks | ||
| function sendMessageToChild(address _receiver, bytes calldata _data) external {} | ||
| } | ||
|
|
||
| contract DepositManagerMock { | ||
| function depositERC20ForUser( | ||
| address token, | ||
| address user, | ||
| uint256 amount // solhint-disable-next-line no-empty-blocks | ||
| ) external {} // solhint-disable-line no-empty-blocks | ||
| } | ||
|
|
||
| contract PolygonRegistryMock { | ||
| // solhint-disable-next-line no-empty-blocks | ||
| function erc20Predicate() external returns (address predicate) {} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know this is a mock, but it looks like using the "registry" from polygon's list may give the wrong value. The Registry is 0x33a02E6cC863D393d6Bf231B697b82F6e499cA71, and you can see that function Again since this is a mock, you can have it return whatever value you want. So not a problem. But it might lead to someone thinking that the actual mainnet contract will return the "deposit" predicate, which it doesn't. I believe the deposit predicate is 0x40ec5b33f54e0e8a33a975908c5ba1c14e5bbbdf. I'm thinking that Polygon_Adapter.sol is deployed on ETH mainnet, for transferring tokens over to Matic. If that's not true... this doesn't apply |
||
| } | ||
|
|
||
| contract PolygonERC20PredicateMock { | ||
| // solhint-disable-next-line no-empty-blocks | ||
| function startExitWithBurntTokens(bytes calldata data) external {} | ||
| } | ||
|
|
||
| contract PolygonERC20Mock is ERC20 { | ||
| // solhint-disable-next-line no-empty-blocks | ||
| constructor() ERC20("Test ERC20", "TEST") {} | ||
|
|
||
| // solhint-disable-next-line no-empty-blocks | ||
| function withdraw(uint256 amount) external {} | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should
wrapbe triggered on any other external functions likeprocessMessageFromRootfor convenience?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could. Note: the functions that actually would need the tokens aren't triggered by processMessageFromRoot, though. Executing relayer refunds and slow relays are the ones that would require this, but to add it to those, they might need to be overridden.
Will think a little about how to do this with little overhead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this functionality in the last commit: 811ac20. PTAL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah perfect this is now consistent with how
Optimism_SpokePoolhandles the same logic: https://github.com/across-protocol/contracts-v2/blob/master/contracts/Optimism_SpokePool.sol#L89HOWEVER, you can probably save some gas by adding a check
if (destinationToken == weth) ...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really think this extra check is worthwhile because:
address(this).balance > 0, which is extremely cheap. It costs 5 gas according to https://ethereum.org/en/developers/docs/evm/opcodes/ to read the balance. Reading thewethvalue from the contract, by comparison, would cost 100 - 2100 gas. I think these checks are the only difference between the two, since we will likely run some matic execution step each time matic tokens are sent over to the spoke, meaning the number of wraps shouldn't really differ between the two.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed offline, we'll leave this as is.