Skip to content
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

[SC-425] Adding Circle CCTP support #15

Merged
merged 8 commits into from
May 20, 2024
Merged

[SC-425] Adding Circle CCTP support #15

merged 8 commits into from
May 20, 2024

Conversation

hexonaut
Copy link
Contributor

Adding Circle CCTP support. Please note the naming scheme: anything labelled with just "CCTP" means the protocol itself and anything labelled with "CircleCCTP" refers to the instance that Circle controls.

CCTP is a general protocol so in theory it could be used by another operator.

@hexonaut hexonaut marked this pull request as ready for review May 10, 2024 08:04
src/CCTPReceiver.sol Outdated Show resolved Hide resolved
test/CCTPIntegration.t.sol Show resolved Hide resolved
@hexonaut hexonaut requested a review from barrutko May 13, 2024 07:29
Copy link
Contributor

@lucas-manuel lucas-manuel left a comment

Choose a reason for hiding this comment

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

Lets have a discussion on naming for this, logic is looking good though

src/CCTPReceiver.sol Outdated Show resolved Hide resolved
src/testing/CircleCCTPDomain.sol Outdated Show resolved Hide resolved
src/testing/CircleCCTPDomain.sol Outdated Show resolved Hide resolved
src/CCTPReceiver.sol Outdated Show resolved Hide resolved
test/CCTPIntegration.t.sol Outdated Show resolved Hide resolved
src/XChainForwarders.sol Outdated Show resolved Hide resolved
test/CCTPIntegration.t.sol Outdated Show resolved Hide resolved
Copy link
Contributor

@barrutko barrutko left a comment

Choose a reason for hiding this comment

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

I'd go a step further in changing the naming from host/bridged to source/destination and only kept relayFromHost and relayToHost for compatibility reasons

src/CCTPReceiver.sol Outdated Show resolved Hide resolved
src/testing/CircleCCTPDomain.sol Show resolved Hide resolved
src/testing/CircleCCTPDomain.sol Show resolved Hide resolved
test/CCTPIntegration.t.sol Outdated Show resolved Hide resolved
test/CCTPIntegration.t.sol Show resolved Hide resolved
test/CCTPIntegration.t.sol Show resolved Hide resolved
src/testing/CircleCCTPDomain.sol Outdated Show resolved Hide resolved
src/CCTPReceiver.sol Outdated Show resolved Hide resolved
src/CCTPReceiver.sol Outdated Show resolved Hide resolved
src/XChainForwarders.sol Outdated Show resolved Hide resolved
Copy link

Coverage after merging SC-425-cctp-support into master will be

63.64%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   ArbitrumReceiver.sol80%100%75%75%15
   CCTPReceiver.sol80%90%75%72.73%19–21, 43
   GnosisReceiver.sol77.78%100%75%62.50%24–26
   OptimismReceiver.sol84.62%100%75%80%21
   XChainForwarders.sol50%100%50%50%122, 139, 169, 186, 198, 241, 70, 83

@hexonaut
Copy link
Contributor Author

Guys regarding the L1/L2 thing, I've added a new task to refactor this repo and generalize it to a multi-bridge world. CCTP is the first case where we are not using a "canonical bridge", but this trend will probably continue and there are now cases where we need to bridge L2 to L1 such as the allocator module which needs to bridge L2 USDC back to Ethereum. It makes sense to refactor this repo and drop the adherence to L1/L2, domains being attached to 1 particular bridge, etc. So let's merge this in and the refactor can be addressed in a new PR.

@hexonaut hexonaut merged commit d71b629 into master May 20, 2024
2 of 3 checks passed
@hexonaut hexonaut deleted the SC-425-cctp-support branch May 20, 2024 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants