Skip to content

Add an ERC7802Bridge #174

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

Closed
wants to merge 13 commits into from
Closed

Add an ERC7802Bridge #174

wants to merge 13 commits into from

Conversation

Amxx
Copy link
Collaborator

@Amxx Amxx commented Jun 12, 2025

Based on #171

@Amxx Amxx requested review from cjcobb23 and a team as code owners June 12, 2025 16:02
@Amxx Amxx marked this pull request as draft June 12, 2025 16:08
@Amxx Amxx marked this pull request as ready for review June 25, 2025 12:27
}
}

abstract contract ERC7802BridgeCustody is ERC7802Bridge {
Copy link
Collaborator Author

@Amxx Amxx Jun 26, 2025

Choose a reason for hiding this comment

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

Suggested change
abstract contract ERC7802BridgeCustody is ERC7802Bridge {
abstract contract ERC20CustodialBridge is ERC7802Bridge {

}

// TODO: check that bridge includes an address ? prevent override ?
function registerRemote(address token, bytes memory bridge, bytes memory remote) public virtual onlyOwner {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TODO: onlyOwnerOrToken modifier.

@@ -0,0 +1,161 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.27;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pragma solidity ^0.8.27;
pragma solidity ^0.8.26;

Comment on lines +31 to +32
mapping(address token => mapping(bytes chain => bytes bridge)) private _remoteBridges;
mapping(address token => mapping(bytes chain => bytes remote)) private _remoteTokens;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
mapping(address token => mapping(bytes chain => bytes bridge)) private _remoteBridges;
mapping(address token => mapping(bytes chain => bytes remote)) private _remoteTokens;
mapping(address token => mapping(bytes chain => bytes)) private _remoteBridges;
mapping(address token => mapping(bytes chain => bytes)) private _remoteTokens;

require(remote.length > 0, ERC7802BridgeMissingRemote(token, chain));
}

// TODO: check that gateway is not address(0) ? prevent override ?
Copy link
Member

Choose a reason for hiding this comment

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

If we're checking that gateway is not address(0) here I don't think we should check it on the getGateway. I'm generally unsure whether to check address(0) in the first place.

require(msg.sender == getGateway(srcChain), ERC7802BridgeInvalidGateway());

// identify local token (requested for mint) and validate sender is the correct bridge
(, address token) = local.parseEvmV1(); // todo: check chainid ?
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 we should indeed check chainId. One can register a bridge that may have the same address in another chain (perhaps different code), so getRemoteBridge will return true.

}

function _distributeTokens(address token, address to, uint256 amount) internal virtual {
IERC7802(token).crosschainMint(to, amount);
Copy link
Member

Choose a reason for hiding this comment

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

Shall we validate that to is not the gateway itself? Otherwise we should add a sweep function (not sure if I like the sweep function though)

}
}

abstract contract ERC7802BridgeCustody is ERC7802Bridge {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this version is necessary if the main bridge can receive tokens itself. Otherwise, I would call this one an ERC7802Passive since the main difference is whether tokens are actively sent to a recipient or passively claimed by the recipient

@Amxx Amxx closed this Jul 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants