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

CCIP-3736 FeeQuoter gas optimizations and redundancy/sanity checks #14805

Merged
merged 33 commits into from
Oct 24, 2024
Merged

CCIP-3736 FeeQuoter gas optimizations and redundancy/sanity checks #14805

merged 33 commits into from
Oct 24, 2024

Conversation

jhweintraub
Copy link
Collaborator

@jhweintraub jhweintraub commented Oct 16, 2024 •
edited
Loading

Misc. Gas optimizations and sanity check fixes for FeeQuoter and other CCIP-related contracts.

  1. FeeQuoter FeeTokenConfig now enforces that minFeeUSDCents < MaxFeeUSDCents and throws an error if not
  2. comment cleanups to be more descriptive
  3. error UnsupportedNumberOfTokens() now includes parameters on included number and max number of supported tokens
  4. Gas optimization that saves gas by not checking tokenTransferFeeConfig.destGasOverhead unless the transferFeeConfig is enabled or not

@jhweintraub jhweintraub changed the title gas optimizations and redundancy/sanity checks CCIP-3736 gas optimizations and redundancy/sanity checks Oct 16, 2024
@github-actions GitHub Actions
Copy link
Contributor

AER Report: CI Core

aer_workflow , commit , Detect Changes , Clean Go Tidy & Generate , Core Tests (go_core_tests) , Core Tests (go_core_ccip_deployment_tests) , Core Tests (go_core_race_tests) , lint , Core Tests (go_core_fuzz) , Flakey Test Detection , SonarQube Scan

1. Service Temporarily Unavailable:re-run tests

Source of Error: 
Re-run tests	2024-10-16T15:59:41.3159503Z 2024/10/16 15:59:41 Error re-running flakey tests: push request failed: status=503, body=<html>
Re-run tests	2024-10-16T15:59:41.3161642Z <head><title>503 Service Temporarily Unavailable</title></head>
Re-run tests	2024-10-16T15:59:41.3169380Z <body>
Re-run tests	2024-10-16T15:59:41.3170970Z <center><h1>503 Service Temporarily Unavailable</h1></center>
Re-run tests	2024-10-16T15:59:41.3171883Z <hr><center>nginx</center>
Re-run tests	2024-10-16T15:59:41.3172402Z </body>
Re-run tests	2024-10-16T15:59:41.3172846Z </html>
Re-run tests	2024-10-16T15:59:41.3186536Z ##[error]Process completed with exit code 1.

Why: The error occurred because the service that the push request was sent to is temporarily unavailable, as indicated by the 503 status code. This is typically a server-side issue.

Suggested fix: Retry the request after some time. If the issue persists, check the server status and logs to identify and resolve the underlying cause of the service unavailability.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 17, 2024 •
edited
Loading

AER Report: CI Core

aer_workflow , commit , Detect Changes , Clean Go Tidy & Generate , Core Tests (go_core_tests) , Core Tests (go_core_ccip_deployment_tests) , Core Tests (go_core_race_tests) , lint , Core Tests (go_core_fuzz) , Flakey Test Detection , SonarQube Scan

1. Service Temporarily Unavailable:re-run tests

Source of Error: 
Re-run tests	2024-10-16T15:59:41.3159503Z 2024/10/16 15:59:41 Error re-running flakey tests: push request failed: status=503, body=<html>
Re-run tests	2024-10-16T15:59:41.3161642Z <head><title>503 Service Temporarily Unavailable</title></head>
Re-run tests	2024-10-16T15:59:41.3169380Z <body>
Re-run tests	2024-10-16T15:59:41.3170970Z <center><h1>503 Service Temporarily Unavailable</h1></center>
Re-run tests	2024-10-16T15:59:41.3171883Z <hr><center>nginx</center>
Re-run tests	2024-10-16T15:59:41.3172402Z </body>
Re-run tests	2024-10-16T15:59:41.3172846Z </html>
Re-run tests	2024-10-16T15:59:41.3186536Z ##[error]Process completed with exit code 1.

Why: The error occurred because the service that the push request was sent to is temporarily unavailable, as indicated by the 503 status code. This is typically a server-side issue.

Suggested fix: Retry the request after some time. If the issue persists, check the server status and logs to identify and resolve the underlying cause of the service unavailability.

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 17, 2024 •
edited
Loading

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 17, 2024 •
edited
Loading

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

@jhweintraub jhweintraub requested a review from a team as a code owner October 17, 2024 15:55
@jhweintraub jhweintraub requested a review from Atrax1 October 17, 2024 15:55
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 17, 2024 •
edited
Loading

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

@jhweintraub jhweintraub requested a review from a team as a code owner October 17, 2024 16:21
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 17, 2024 •
edited
Loading

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 17, 2024 •
edited
Loading

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 17, 2024 •
edited
Loading

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 23, 2024 •
edited
Loading

Solidity Review Jira issue

Hey! We have taken the liberty to link this PR to a Jira issue for Solidity Review.

This is a new feature, that's currently in the pilot phase, so please make sure that the linkage is correct. In a contrary case, please update it manually in JIRA and replace Solidity Review issue key in the changeset file with the correct one.
Please reach out to the Test Tooling team and notify them about any issues you encounter.

Any changes to the Solidity Review Jira issue should be reflected in the changeset file. If you need to update the issue key, please do so manually in the following changeset file: contracts/.changeset/happy-crabs-rescue.md

This PR has been linked to Solidity Review Jira issue: CCIP-3966

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 23, 2024 •
edited
Loading

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 23, 2024 •
edited
Loading

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

RensR
RensR reviewed Oct 24, 2024
View reviewed changes
.changeset/sixty-jobs-retire.md Outdated
"chainlink": patch
---

FeeQuoter comment updates and sanity checks to avoid ownership abuse/gas savings #bugfix
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not going to block on it but this doesn't need a core changeset anymore :D

RensR
RensR previously approved these changes Oct 24, 2024
View reviewed changes
@RensR
Merge branch 'develop' into feeQuoter_misc_optimizations
99c9073 
# Conflicts:
#	contracts/gas-snapshots/ccip.gas-snapshot
#	core/gethwrappers/ccip/generated/fee_quoter/fee_quoter.go
#	core/gethwrappers/ccip/generation/generated-wrapper-dependency-versions-do-not-edit.txt
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 24, 2024 •
edited
Loading

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

RensR
RensR previously approved these changes Oct 24, 2024
View reviewed changes
@github-actions GitHub Actions
Copy link
Contributor

Static analysis results are available

Hey @RensR, you can view Slither reports in the job summary here or download them as artifact here.
Please check them before merging and make sure you have addressed all issues.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 24, 2024 •
edited
Loading

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

@cl-sonarqube-production
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@RensR RensR added this pull request to the merge queue Oct 24, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 24, 2024
@RensR RensR added this pull request to the merge queue Oct 24, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 24, 2024
@RensR RensR added this pull request to the merge queue Oct 24, 2024
Merged via the queue into develop with commit b17c09d Oct 24, 2024
170 of 171 checks passed
@RensR RensR deleted the feeQuoter_misc_optimizations branch October 24, 2024 15:49
KMontag42 pushed a commit that referenced this pull request Oct 24, 2024
@jhweintraub @app-token-issuer-infra-releng
@RensR @KMontag42
CCIP-3736 FeeQuoter gas optimizations and redundancy/sanity checks (#…
2d6d1f9 
…14805)

* gas optimizations and redundancy/sanity checks

* Update gethwrappers

* formatting, shapshotting, and changesetting

* CCIP-3736 update foundry and snapshot again

* [Bot] Update changeset file with jira issues

* Update gethwrappers

* snapshot fix and missing chageset file

* snapshot CI tests are the bane of my existence

* fill in coverage gap

* forge fmt

* Update gethwrappers

* CI checks are murphy's law, anything can happen at any time for no reason

* Update gethwrappers

* gas fix

* i will never understand why the gas changes on every single run.

* Update FeeQuoter.sol

* [Bot] Update changeset file with jira issues

* gas snapshot update after merge

* Update gethwrappers

* rm changeset

---------

Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
Co-authored-by: Rens Rooimans <[email protected]>
@chainchad chainchad mentioned this pull request Nov 1, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RayXpub RayXpub RayXpub approved these changes

@RensR RensR RensR approved these changes

@matYang matYang Awaiting requested review from matYang

@0xsuryansh 0xsuryansh Awaiting requested review from 0xsuryansh

@RyanRHall RyanRHall Awaiting requested review from RyanRHall

@Atrax1 Atrax1 Awaiting requested review from Atrax1 Atrax1 was automatically assigned from smartcontractkit/foundations

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

Successfully merging this pull request may close these issues.
3 participants
@jhweintraub @RensR @RayXpub