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.

Sign up for GitHub

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

Jump to bottom

feat: add polymer contracts for ERC-7683 #92

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add polymer contracts for ERC-7683 #92

wants to merge 1 commit into from

Conversation

zhengyangfeng00
Copy link

@zhengyangfeng00 zhengyangfeng00 commented Feb 13, 2025 •
edited
Loading

Description

Add implementation of ERC-7683 using Polymer as the messaging layer for cross-chain event verification. This PR introduces:

  • New Polymer7683 contract that extends BasicSwap7683 with Polymer protocol integration
  • Integration with CrossL2ProverV2 for secure cross-chain event verification
  • Support for handling settlement and refund proofs for cross-chain orders
  • Prevention of replay attacks through event hash tracking
  • High unit test coverage

Drive-by changes

None

Related issues

None

Backward compatibility

Yes

Testing

Unit tests

❯ forge test --match-path test/Polymer7683.t.sol
[⠊] Compiling...
No files changed, compilation skipped

Ran 10 tests for test/Polymer7683.t.sol:Polymer7683Test
[PASS] test_handleRefundWithProof_success() (gas: 304358)
[PASS] test_handleSettlementWithProof_invalidOrderIds() (gas: 373255)
[PASS] test_handleSettlementWithProof_multipleBatch() (gas: 565722)
[PASS] test_handleSettlementWithProof_preventReplay() (gas: 385507)
[PASS] test_handleSettlementWithProof_success() (gas: 377122)
[PASS] test_handleSettlementWithProof_unregisteredDestination() (gas: 313861)
[PASS] test_handleSettlementWithProof_wrongChainId() (gas: 348528)
[PASS] test_handleSettlementWithProof_wrongEmitter() (gas: 350039)
[PASS] test_setDestinationContract() (gas: 42927)
[PASS] test_setDestinationContract_onlyOwner() (gas: 16905)
Suite result: ok. 10 passed; 0 failed; 0 skipped; finished in 3.63ms (5.24ms CPU time)

Ran 1 test suite in 189.28ms (3.63ms CPU time): 10 tests passed, 0 failed, 0 skipped (10 total tests)

@vercel Vercel
Copy link

vercel bot commented Feb 13, 2025

@zhengyangfeng00 is attempting to deploy a commit to the BootNode Team on Vercel.

A member of the Team first needs to authorize it.

@zhengyangfeng00 zhengyangfeng00 force-pushed the zfeng/polymer branch 2 times, most recently from ebc0d2d to ac9fc34 Compare February 13, 2025 18:18
@zhengyangfeng00 zhengyangfeng00 marked this pull request as draft February 13, 2025 19:13
@zhengyangfeng00 zhengyangfeng00 marked this pull request as ready for review February 13, 2025 19:17
lmcorbalan
lmcorbalan reviewed Feb 14, 2025
View reviewed changes
lmcorbalan
lmcorbalan reviewed Feb 20, 2025
View reviewed changes
solidity/src/Polymer7683.sol Outdated
Comment on lines 182 to 185
OrderData memory orderData = OrderEncoder.decode(originData);
if (provenChainId != orderData.destinationDomain) {
revert InvalidChainId();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

good catch!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks to this catch we realized there were a potential vulnerability in the our base contacts and we addressed it with this changes #112
Please notice the changes on both _handleSettleOrder and _handleRefundOrder which now receceive the msg origin and msg sender of the settle and refund, which in you case would be the provenChainId and actualEmitter respectively.
If I'm not wrong the fix should help simplify this and _validateRefundProof

Copy link
Author

Choose a reason for hiding this comment

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

yes, i've rebased this PR on #112 and refactored the code to simplify.

lmcorbalan
lmcorbalan reviewed Feb 25, 2025
View reviewed changes
solidity/src/Polymer7683.sol Outdated
* @param eventProof The proof of the Fill event from the destination chain
*/
function handleSettlementWithProof(
bytes32 orderId,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you don't need this, unlike the handleRefundWithProof where you need the orderId bc the event you use emits for a batch orders

solidity/src/Polymer7683.sol Outdated
Comment on lines 182 to 185
OrderData memory orderData = OrderEncoder.decode(originData);
if (provenChainId != orderData.destinationDomain) {
revert InvalidChainId();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks to this catch we realized there were a potential vulnerability in the our base contacts and we addressed it with this changes #112
Please notice the changes on both _handleSettleOrder and _handleRefundOrder which now receceive the msg origin and msg sender of the settle and refund, which in you case would be the provenChainId and actualEmitter respectively.
If I'm not wrong the fix should help simplify this and _validateRefundProof

@zhengyangfeng00 zhengyangfeng00 changed the base branch from main to refund-wrong-chain February 26, 2025 21:07
lmcorbalan
lmcorbalan reviewed Mar 5, 2025
View reviewed changes
lmcorbalan
lmcorbalan reviewed Mar 5, 2025
View reviewed changes
@lmcorbalan lmcorbalan deleted the branch BootNodeDev:main March 11, 2025 12:12
@lmcorbalan lmcorbalan closed this Mar 11, 2025
lmcorbalan
lmcorbalan reviewed Mar 12, 2025
View reviewed changes
@lmcorbalan lmcorbalan reopened this Mar 12, 2025
@lmcorbalan lmcorbalan changed the base branch from refund-wrong-chain to main March 12, 2025 12:18
lmcorbalan
lmcorbalan approved these changes Mar 18, 2025
View reviewed changes
Copy link
Collaborator

@lmcorbalan lmcorbalan left a comment

Choose a reason for hiding this comment

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

It would be good to cover the missing branches with tests.
Other than that and the comment related to the error the PR looks good to me.

solidity/src/Polymer7683.sol
);

// Check if order was successfully refunded
if (orderStatus[orderId] != REFUNDED) revert SettlementFailed();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this error meant to be the same as the settlement function?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lmcorbalan lmcorbalan lmcorbalan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zhengyangfeng00 @lmcorbalan