-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
CCIP-3730 Misc ccip onchain changes #14845
Conversation
I see you updated files related to
|
7180423
to
33a147e
Compare
@@ -28,7 +28,7 @@ jobs: | |||
cat <<EOF > matrix.json | |||
[ | |||
{ "name": "automation", "setup": { "run-coverage": false, "min-coverage": 98.5, "run-gas-snapshot": false, "run-forge-fmt": false }}, | |||
{ "name": "ccip", "setup": { "run-coverage": true, "min-coverage": 97.6, "run-gas-snapshot": true, "run-forge-fmt": true }}, | |||
{ "name": "ccip", "setup": { "run-coverage": true, "min-coverage": 98.8, "run-gas-snapshot": true, "run-forge-fmt": true }}, |
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.
this is in line with current cov (98.81%)
import {ITypeAndVersion} from "../shared/interfaces/ITypeAndVersion.sol"; | ||
import {IFeeQuoter} from "./interfaces/IFeeQuoter.sol"; | ||
import {IPriceRegistry} from "./interfaces/IPriceRegistry.sol"; | ||
|
||
import {KeystoneFeedsPermissionHandler} from "../keystone/KeystoneFeedsPermissionHandler.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.
keystone is not third party
// │ Extra data availability bytes that are returned from the source pool and sent | ||
uint32 destBytesOverhead; // │ to the destination pool. Must be >= Pool.CCIP_LOCK_OR_BURN_V1_RET_BYTES | ||
bool isEnabled; // ─────────────────╯ Whether this token has custom transfer fees | ||
uint32 minFeeUSDCents; // ────╮ Minimum fee to charge per token transfer, multiples of 0.01 USD |
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.
there's no reason so add long ──── lines. At most 2 on the shortest leg of these comments
AER Report: CI Core ran successfully ✅ |
AER Report: CI Core ran successfully ✅AER Report: Operator UI CI ran successfully ✅ |
@@ -232,7 +232,7 @@ abstract contract OCR3Base is OwnerIsCreator, OCR3Abstract { | |||
// Scoping this reduces stack pressure and gas usage | |||
{ | |||
uint256 expectedDataLength = uint256(TRANSMIT_MSGDATA_CONSTANT_LENGTH_COMPONENT) + | |||
report.length + // one byte pure entry in _report |
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.
😆
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 👍
…ue-get-allowlist # Conflicts: # contracts/gas-snapshots/ccip.gas-snapshot # core/gethwrappers/ccip/generation/generated-wrapper-dependency-versions-do-not-edit.txt
4b90ba2
AER Report: CI Core ran successfully ✅AER Report: Operator UI CI ran successfully ✅ |
Quality Gate passedIssues Measures |
* improve return value get allowlist * update order of applyFeeTokensUpdates improve tests * consistent naming of allowlist * fix typo, dedup error * improve comments * fix offchain * changeset + fix lint * remove non-allowed state transition * fix naming of allowListUpdates * re run CI
* develop: rm gethwrappers from codeowners offchain (#14919) CCIP-3899 fix sender encoding (#14877) ccip: use unknown address type. (#14836) [chore] Add changeset entry for RequestRoundTracker fix (#14912) various Solidity Foundry updates (#14866) CCIP-3710 create new custom calldata L1 (DA) gas oracle (#14710) CCIP owns smoke test (#14906) core/services/ocr2/plugins/ccip/internal/ccipdata/factory: check error from TypeAndVersion (#14861) CCIP-3730 Misc ccip onchain changes (#14845) Chain fee integration tests (#14829) release/2.17.0 -> develop (#14721) Solana: re-enable disabled test with updated version (#14892)
* improve return value get allowlist * update order of applyFeeTokensUpdates improve tests * consistent naming of allowlist * fix typo, dedup error * improve comments * fix offchain * changeset + fix lint * remove non-allowed state transition * fix naming of allowListUpdates * re run CI
Fixes various comments, imports and typo's.
Most impactful change is fixing the ordering of _applyFeeTokensUpdates, with removes first to be in line with the other code.