Add feed chain deployment test #14461

asoliman92 · 2024-09-17T16:28:36Z

Start with token_info which has the config map of TokenDescriptor to tokenInfo that will be used in commit off chain plugin.

In production the token_info will be populated with all token descriptors we're interested in with their respective on-chain aggregator address.
In tests we do 2 things:
- We Deploy feeds and add them to tokenInfo with the correct descriptors (Assuming for now that same token won't have different info across different chains)
- Once we deploy tokens on simulated chain (e.g. LinkToken). We use GetTokenInfo to map the token address on dest chain to the info added in previous step.
Add function DeployFeed to test_helpers to deploy mock feeds while testing.

* Add deploy_feed_chain * Refactor deployCapReg to take the chain directly without the selector * Deploy mock link feed

connorwstein · 2024-09-17T18:10:34Z

integration-tests/deployment/ccip/deploy_feed_chain.go

+	LINK_PRICE = big.NewInt(5e18)
+)
+
+func DeployFeeds(lggr logger.Logger, chain deployment.Chain) (deployment.AddressBook, map[string]common.Address, error) {


just to clarify this would only be for testing - on mainnet we'd leverage existing feed deployment stack

connorwstein · 2024-09-17T18:15:37Z

integration-tests/deployment/ccip/deploy_feed_chain.go

+func DeployFeeds(lggr logger.Logger, chain deployment.Chain) (deployment.AddressBook, map[string]common.Address, error) {
+	ab := deployment.NewMemoryAddressBook()
+	//TODO: Maybe append LINK to the contract name
+	linkTV := deployment.NewTypeAndVersion(PriceFeed, deployment.Version1_0_0)


hm this should be whatever typeAndVersion is actually in the contract - does the mock aggregator not have one?

mock aggregator doesn't have one. That's why I added the TODO to make it unique with different aggregators later.

connorwstein · 2024-09-17T18:19:19Z

integration-tests/deployment/ccip/state.go

+			if state.Feeds == nil {
+				state.Feeds = make(map[string]*aggregator_v3_interface.AggregatorV3Interface)
+			}
+			state.Feeds[tvStr.String()] = feed


I don't think tvStr will be unique here, they'll all be of the type OffchainAggregator (https://github.com/smartcontractkit/libocr/blob/1a7e0789ba965e9a1cdfe6fb4d5e31acd1627025/contract/OffchainAggregator.sol#L112) or something. Maybe best to key them on description() e.g. "LINK / USD" see an example here of a real feed https://sepolia.arbiscan.io/address/0x0FB99723Aee6f420beAD13e6bBB79b7E6F034298#readContract

I like the idea of using description 👍. I was going to prefix it with the token name but description is cleaner in this case.

connorwstein · 2024-09-17T18:20:08Z

integration-tests/deployment/ccip/deploy.go

@@ -119,6 +122,7 @@ func deployContract[C Contracts](

 type DeployCCIPContractConfig struct {
 	HomeChainSel   uint64
+	FeedChainSel   uint64


I think since on mainnet/testnet we would not deploy the feeds (mock or o/w), probably best to leave it out of the DeployCCIPContracts function and just keep it separate which we only deploy in tests.

I need to pass it as the deployment offchain config needs it

token_info facilitates sending tokenConfig to CCIPDeployConfig

integration-tests/deployment/ccip/consts.go

integration-tests/deployment/ccip/test_helpers.go

integration-tests/deployment/ccip/state.go

integration-tests/deployment/ccip/test_helpers.go

makramkd · 2024-09-18T17:36:35Z

integration-tests/deployment/ccip/test_helpers.go

+				DECIMALS,      // decimals
+				MockLinkPrice, // initialAnswer
+			)
+			aggregatorCr, err2 := aggregator_v3_interface.NewAggregatorV3Interface(linkFeed, chain.Client)


This code is unidiomatic; typically this NewAggregatorV3Interface will never return an error unless the ABI is not well formed (which is impossible - guaranteed by the compilation process). Better to check err immediately.

We're inside a function that's passed as a parameter and needs to return ContractDeploy[*aggregator_v3_interface.AggregatorV3Interface]{. You suggest just return the ContractDeploy twice with different errors after each?

hm might be simpler to just have MockFeeds map[TokenSymbol]*mock_aggregator_v3.MockAggregator then when we add real feeds separately have the full OffchainAggregator wrapper

We'll give away the flexibility of the tests though with deploying any mock contract by description.

makramkd · 2024-09-18T17:37:23Z

integration-tests/deployment/ccip/test_helpers.go

+		return ab, nil, err
+	}
+
+	if desc != MockLinkAggregatorDescription {


Having a proper description would make this assertion more useful, cuz right now the description of a mock link feed and a mock weth feed will be the same right?

Nope, I'm gonna use this: MockWETHAggregatorDescription = "MockETHUSDAggregator". Didn't want to use same with same description as it will cause issues.

chainlink/contracts/src/v0.8/automation/testhelpers/MockETHUSDAggregator.sol

Line 19 in a348b98

return "MockETHUSDAggregator";

connorwstein · 2024-09-18T18:56:20Z

integration-tests/deployment/ccip/deploy_home_chain.go

@@ -367,11 +370,15 @@ func AddDON(
 	capReg *capabilities_registry.CapabilitiesRegistry,
 	ccipConfig *ccip_config.CCIPConfig,
 	offRamp *offramp.OffRamp,
+	feedChainSel uint64,
+	// Token on Dest chain to aggregate address on feed chain
+	//tokenCfg TokenConfig,


remove commented code?

connorwstein · 2024-09-18T19:03:43Z

integration-tests/deployment/ccip/state.go

+	// Map between token Descriptor (e.g. LinkSymbol, WethSymbol)
+	// and the respective token contract
+	// This is more of an illustration of how we'll have tokens, and it might need some work later to work properly.
+	BurnMintTokens677 map[TokenSymbol]*burn_mint_erc677.BurnMintERC677


are these intended to non-fee priced tokens like custom BPS tokens (fee tokens should already be captured with the Weth9 and LinkToken wrappers)? If so they won't necessarily be B/M 677

are these intended to non-fee priced tokens like custom BPS tokens

This was the intention. What are your suggestions? We can have multiple maps for different kinds as well. For now this is for illustrative purpose. Maybe I can add more comments to explain this.

This doesn't seem to be used, any reason we have them here?

integration-tests/deployment/ccip/test_helpers.go

connorwstein · 2024-09-18T19:10:54Z

integration-tests/deployment/ccip/test_helpers.go

+				DECIMALS,      // decimals
+				MockLinkPrice, // initialAnswer
+			)
+			aggregatorCr, err2 := aggregator_v3_interface.NewAggregatorV3Interface(linkFeed, chain.Client)


hm might be simpler to just have MockFeeds map[TokenSymbol]*mock_aggregator_v3.MockAggregator then when we add real feeds separately have the full OffchainAggregator wrapper

integration-tests/deployment/ccip/state.go

integration-tests/deployment/ccip/test_assertions.go

integration-tests/deployment/ccip/token_info.go

makramkd · 2024-09-19T18:07:08Z

integration-tests/deployment/ccip/test_assertions.go

+						t.Logf("Received commit report on selector %d from source selector %d expected seq nr range %s, token prices: %v",
+							dest.Selector, src.Selector, expectedSeqNumRange.String(), report.Report.PriceUpdates.TokenPriceUpdates)


Do we want to have at least one assertion per chain that a token price gets posted? I don't see any assertions right now regarding this.

We do assert per chain in the main tests. This is just for printing no more.

https://github.com/smartcontractkit/chainlink/pull/14461/files#diff-44a3f004be4e7af8fc5fed7284207bd3a24a3d5ca9e016cab6662a45830121e1R88-R97

cl-sonarqube-production · 2024-09-20T13:39:59Z

Quality Gate passed

Issues
3 New issues
2 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube

asoliman92 added 2 commits September 17, 2024 20:28

Initial Commit

bdbcea4

* Add deploy_feed_chain * Refactor deployCapReg to take the chain directly without the selector * Deploy mock link feed

Add merge method to State

a96e5a3

asoliman92 changed the title ~~Initial Commit~~ Add feed chain deployment Sep 17, 2024

asoliman92 added 2 commits September 17, 2024 21:12

Use AddressBook merge

9d929fc

Deploy feed chain in NewEnvironment helper

a348b98

connorwstein reviewed Sep 17, 2024

View reviewed changes

asoliman92 added 6 commits September 18, 2024 13:15

Move DeployFeeds into test helpers

f95960d

Add token_info.go

fdb2305

token_info facilitates sending tokenConfig to CCIPDeployConfig

Merge branch 'develop' into feed-chain-deployment

a3725e4

go get cl-ccip with reader enabled

fdbcc19

wip assert token price reported

1e19e24

make gomodtidy

e6aaa80

asoliman92 marked this pull request as ready for review September 18, 2024 17:35

asoliman92 requested review from a team as code owners September 18, 2024 17:35

asoliman92 requested review from EasterTheBunny and removed request for a team September 18, 2024 17:35

Merge branch 'develop' into feed-chain-deployment

a121f21

asoliman92 enabled auto-merge September 18, 2024 17:35

asoliman92 requested a review from connorwstein September 18, 2024 17:35

makramkd reviewed Sep 18, 2024

View reviewed changes

asoliman92 added 2 commits September 18, 2024 22:09

Change Descriptor to Symbol

c77e128

fix linting and add changeset

e6c11f2

connorwstein reviewed Sep 18, 2024

View reviewed changes

asoliman92 added 4 commits September 19, 2024 19:24

Change Feeds to USDFeeds

1842c22

Assert token prices by calling FeeQuoter after the commit report

bb213f8

Test token prices in add_chain

3976f6d

Merge branch 'develop' into feed-chain-deployment

9becdc5

bump cl-ccip version

950aa26

makramkd reviewed Sep 19, 2024

View reviewed changes

Remove unused code from token_info.go

116d48b

makramkd previously approved these changes Sep 19, 2024

View reviewed changes

fix linting

11ee5db

asoliman92 dismissed makramkd’s stale review via 11ee5db September 20, 2024 12:13

asoliman92 and others added 2 commits September 20, 2024 16:46

fix linting

60e8b11

Merge branch 'develop' into feed-chain-deployment

3c6875f

connorwstein approved these changes Sep 20, 2024

View reviewed changes

asoliman92 changed the title ~~Add feed chain deployment~~ Add feed chain deployment test Sep 20, 2024

winder approved these changes Sep 20, 2024

View reviewed changes

asoliman92 added this pull request to the merge queue Sep 20, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 20, 2024

asoliman92 added this pull request to the merge queue Sep 20, 2024

AnieeG approved these changes Sep 20, 2024

View reviewed changes

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 20, 2024

asoliman92 added this pull request to the merge queue Sep 20, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 20, 2024

asoliman92 added this pull request to the merge queue Sep 20, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 20, 2024

asoliman92 added this pull request to the merge queue Sep 20, 2024

Merged via the queue into develop with commit 22a8c99 Sep 20, 2024
156 checks passed

asoliman92 deleted the feed-chain-deployment branch September 20, 2024 15:22

github-actions bot mentioned this pull request Sep 20, 2024

[DO NOT MERGE] Changeset Release Preview - v2.17.0 #13148

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add feed chain deployment test #14461

Add feed chain deployment test #14461

asoliman92 commented Sep 17, 2024 •

edited

Loading

connorwstein Sep 17, 2024

connorwstein Sep 17, 2024

asoliman92 Sep 18, 2024

connorwstein Sep 17, 2024

asoliman92 Sep 18, 2024

connorwstein Sep 17, 2024

asoliman92 Sep 18, 2024

makramkd Sep 18, 2024

asoliman92 Sep 18, 2024

connorwstein Sep 18, 2024

asoliman92 Sep 19, 2024

makramkd Sep 18, 2024

asoliman92 Sep 18, 2024

connorwstein Sep 18, 2024

connorwstein Sep 18, 2024

asoliman92 Sep 19, 2024

makramkd Sep 19, 2024

connorwstein Sep 18, 2024

makramkd Sep 19, 2024

asoliman92 Sep 19, 2024

asoliman92 Sep 19, 2024

cl-sonarqube-production bot commented Sep 20, 2024

		t.Logf("Received commit report on selector %d from source selector %d expected seq nr range %s, token prices: %v",
		dest.Selector, src.Selector, expectedSeqNumRange.String(), report.Report.PriceUpdates.TokenPriceUpdates)

Add feed chain deployment test #14461

Add feed chain deployment test #14461

Conversation

asoliman92 commented Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cl-sonarqube-production bot commented Sep 20, 2024

Quality Gate passed

asoliman92 commented Sep 17, 2024 •

edited

Loading