-
Notifications
You must be signed in to change notification settings - Fork 18
Update Crosschain contract: transition from CAIP-10 to ERC-7930 #171
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
Conversation
Note: |
event Received(bytes32 indexed receiveId, address gateway); | ||
event ExecutionSuccess(bytes32 indexed receiveId); | ||
event ExecutionFailed(bytes32 indexed receiveId); | ||
event GatewayAdded(address indexed gateway); | ||
event GatewayRemoved(address indexed gateway); | ||
event ThresholdUpdated(uint8 threshold); | ||
|
||
error ERC7786AggregatorValueNotSupported(); | ||
error UnsupportedNativeTransfer(); |
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 this error will throw an "Already declared identifier" if used along with AxelarGatewaySource
(i.e. contract A is ERC7786Aggregator, AxelarGatewaySource, ... {}
).
We may want to create a library for 7786-related errors
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.
We can rename that error, but the aggregator is an independant contract that does calls to gateways. The aggregator should NOT inherit from another gateway implementation.
I think the analogy is ERC-4626 <> ERC-20.
ERC-4626 implements the ERC-20 interface, and all the corresponding logic, but you would not do contract A is ERC4626, 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.
Got it, I just don't think this is an aggregator then..., it's more like a redundant bridge
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.
Its going to be renamed anyway. The current marking name is "ERC7786OpenBridge". Makes little sens in my opinion.
I think "multibridge" might be a good option.
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.
More in favor of MultiBridge after past discussions but happy with OpenBridge since marketing has gone with that one.
Co-authored-by: Ernesto García <[email protected]>
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.
LGTM 🚢
Based on #169