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

Configure NonceManager for chain reader and CCIPReader. #1359

Closed
wants to merge 2 commits into from
Closed

Configure NonceManager for chain reader and CCIPReader. #1359

wants to merge 2 commits into from

Conversation

winder
Copy link
Contributor

@winder winder commented Aug 23, 2024 •
edited
Loading

  • Unit test for CCIPReader Nonces function.
  • Configure NonceManager in the chain reader.

@winder winder force-pushed the will/nonce-manager-config branch from 21a8eef to 29267b2 Compare August 23, 2024 19:51
@winder winder force-pushed the will/nonce-manager-config branch from 29267b2 to f15ae71 Compare August 27, 2024 19:57
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 27, 2024 •
edited
Loading

LCOV of commit 5103a08 during Solidity Foundry #7812

Summary coverage rate:
  lines......: 98.6% (2024 of 2053 lines)
  functions..: 96.4% (378 of 392 functions)
  branches...: 90.1% (836 of 928 branches)

Files changed coverage rate: n/a

winder
winder commented Aug 28, 2024
View reviewed changes
go.mod Outdated
@@ -74,7 +74,7 @@ require (
github.com/shopspring/decimal v1.4.0
github.com/smartcontractkit/chain-selectors v1.0.21
github.com/smartcontractkit/chainlink-automation v1.0.4
github.com/smartcontractkit/chainlink-ccip v0.0.0-20240827164549-33f5819d7ddc
github.com/smartcontractkit/chainlink-ccip v0.0.0-20240828135226-207ea3027224
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: switch this to ccip-develop once the branch has been merged.

@winder winder force-pushed the will/nonce-manager-config branch from d826b13 to 080c523 Compare August 28, 2024 15:23
@winder winder force-pushed the will/nonce-manager-config branch from 080c523 to 2ba7364 Compare August 28, 2024 15:29
@winder winder marked this pull request as ready for review August 28, 2024 15:41
@winder winder requested review from a team, elatoskinas and RayXpub as code owners August 28, 2024 15:41
@winder winder changed the title WIP: Set nonce manager config for chain reader. Configure NonceManager for chain reader and CCIPReader. Aug 28, 2024
makramkd
makramkd reviewed Aug 28, 2024
View reviewed changes
contracts/src/v0.8/ccip/test/helpers/CCIPReaderTester.sol
@@ -9,6 +9,7 @@ contract CCIPReaderTester {

mapping(uint64 sourceChainSelector => OffRamp.SourceChainConfig sourceChainConfig) internal s_sourceChainConfigs;
mapping(uint64 destChainSelector => uint64 sequenceNumber) internal s_destChainSeqNrs;
mapping(uint64 sourceChainSelector => mapping(bytes sender => uint64 nonce)) internal s_senderNonce;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think in this particular instance we can actually just deploy the NonceManager contract directly - it doesn't seem to have any dependents and is basically standalone. You only have to reference the authorized callers which would be the address of the private key generated in the test.

The advantage of this is we're testing against the same contract that is going to be deployed in prod so any breakages in ABI there will end up breaking the test, forcing us to fix it.

@winder
Set nonce manager config for chain reader.
f32661d 
WIP

Nonces test.

Update ccip reader nonce tests.

Point chainlink-ccip dep to my branch.

Remove TODO that has been completed.

go mod tidy

Change chainlink-ccip dep back to develop branch.
@winder winder force-pushed the will/nonce-manager-config branch from 21831ba to f32661d Compare August 28, 2024 17:49
@winder winder requested a review from makramkd August 28, 2024 19:48
@cl-sonarqube-production
Copy link

Quality Gate failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube

Catch issues before they fail your Quality Gate with our IDE extension SonarLint SonarLint

@github-actions GitHub Actions
Copy link
Contributor

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Oct 14, 2024
@winder winder closed this Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dimkouv dimkouv dimkouv approved these changes

@makramkd makramkd makramkd approved these changes

@elatoskinas elatoskinas Awaiting requested review from elatoskinas elatoskinas is a code owner

@RayXpub RayXpub Awaiting requested review from RayXpub RayXpub is a code owner

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

Successfully merging this pull request may close these issues.
3 participants
@winder @makramkd @dimkouv