-
Notifications
You must be signed in to change notification settings - Fork 18
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
feat: add polymer contracts for ERC-7683 #92
base: refund-wrong-chain
Are you sure you want to change the base?
feat: add polymer contracts for ERC-7683 #92
Conversation
@zhengyangfeng00 is attempting to deploy a commit to the BootNode Team on Vercel. A member of the Team first needs to authorize it. |
ebc0d2d
to
ac9fc34
Compare
af9ac3e
to
925a0ee
Compare
solidity/src/Polymer7683.sol
Outdated
OrderData memory orderData = OrderEncoder.decode(originData); | ||
if (provenChainId != orderData.destinationDomain) { | ||
revert InvalidChainId(); | ||
} |
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.
good catch!
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.
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
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.
yes, i've rebased this PR on #112 and refactored the code to simplify.
solidity/src/Polymer7683.sol
Outdated
* @param eventProof The proof of the Fill event from the destination chain | ||
*/ | ||
function handleSettlementWithProof( | ||
bytes32 orderId, |
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 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
OrderData memory orderData = OrderEncoder.decode(originData); | ||
if (provenChainId != orderData.destinationDomain) { | ||
revert InvalidChainId(); | ||
} |
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.
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
dd9be88
to
686dc8c
Compare
Description
Add implementation of ERC-7683 using Polymer as the messaging layer for cross-chain event verification. This PR introduces:
Drive-by changes
None
Related issues
None
Backward compatibility
Yes
Testing
Unit tests