-
Notifications
You must be signed in to change notification settings - Fork 3
Feat/int 668/ccip #60
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
base: main
Are you sure you want to change the base?
Conversation
src/bridge/EnsoCCIPReceiver.sol
Outdated
| abi.decode(_message.data, (address, uint256, bytes)); | ||
|
|
||
| uint256 availableGas = gasleft(); | ||
| if (estimatedGas != 0 && availableGas < estimatedGas) { |
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.
@PeterMPhillips let me know what do you think about estimatedGas = 0 to bypass gas check. If not useful I'll remove it
src/bridge/EnsoCCIPReceiver.sol
Outdated
|
|
||
| /// @dev Allowlist of source senders per chain selector. | ||
| /// forge-lint: disable-next-item(mixed-case-variable) | ||
| mapping(uint64 sourceChainSelector => mapping(address sender => bool isAllowed)) private s_allowedSender; |
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.
@PeterMPhillips let me know what do you think about this level of granularity.
I really think we do want to control senders (aka. EnsoShortcuts). Furthermore, the s_allowedSourceChain would be superseded by s_allowedSender (more granular).
s_allowedSourceChain and s_allowedSender can both act like "paused" logic (although both with side effects for in-flight messages). However, we may be interested in proper "pausable" logic as pausing would require a single tx vs N txs (1 per supported senders by chain). In fact it seems that Chainlink updated router address at least once (well, just confirmed that it was on testnet).
| v4-periphery-4.0.0/=dependencies/v4-periphery-4.0.0/src/ | ||
| chainlink-ccip=dependencies/chainlink-ccip-1.6.2/chains/evm/contracts/ | ||
| chainlink-ccip-1.6.2/=dependencies/chainlink-ccip-1.6.2/chains/evm/contracts/ | ||
| @openzeppelin/[email protected]/utils/introspection/IERC165.sol=dependencies/@openzeppelin-contracts-5.2.0/contracts/utils/introspection/IERC165.sol |
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.
@PeterMPhillips this remapping is a bit hacky but I wanted to solve Chainlink CCIPReceiver.sol dependency to OZ 5.0.2 without having to install 5.0.2 (IERC165.sol doesn't change between 5.0.2 and 5.2.0 - what we have installed). No problem installing OZ 5.0.2.
98b3088 to
afd501c
Compare
80f94e7 to
3fed934
Compare
c701cb2 to
3975f88
Compare
2b762f1 to
60a5ca5
Compare
60a5ca5 to
cec55b2
Compare
No description provided.