CCIP-3552 refactor OP oracle to accept generic DA oracle config #14599

ogtownsend · 2024-09-27T22:45:22Z

This PR:

adds a DAOracle config to the GasEstimator interface to be set by the chain TOML
refactors the OP oracle to accept this generic DA oracle config

See the CCIP-3551 epic for more details

core/chains/evm/config/config.go

core/chains/evm/config/toml/config.go

ogtownsend · 2024-10-04T18:01:15Z

core/chains/evm/gas/rollups/l1_oracle.go

-	switch chainType {
-	case chaintype.ChainOptimismBedrock, chaintype.ChainKroma, chaintype.ChainScroll, chaintype.ChainMantle, chaintype.ChainZircuit:
-		l1Oracle, err = NewOpStackL1GasOracle(lggr, ethClient, chainType)
-	case chaintype.ChainArbitrum:
+	switch daOracle.OracleType() {
+	case toml.OPOracle:
+		l1Oracle, err = NewOpStackL1GasOracle(lggr, ethClient, chainType, daOracle)
+	case toml.ArbitrumOracle:
 		l1Oracle, err = NewArbitrumL1GasOracle(lggr, ethClient)
-	case chaintype.ChainZkSync:
+	case toml.ZKSyncOracle:
 		l1Oracle = NewZkSyncL1GasOracle(lggr, ethClient)
 	default:
-		return nil, fmt.Errorf("received unsupported chaintype %s", chainType)
+		return nil, fmt.Errorf("unsupported DA oracle type %s", daOracle.OracleType())


This change was made per @matYang 's suggestion here: #14599 (comment)

The one "gotcha" with this is that now we need to make sure the DAOracle config is correctly set and non-nil for each of these chains.

I see how this simplifies this switch but I'm worried about introducing another config for this when ChainType is already used to group behavior for different chains. Don't think this saves us from making changes either. If we introduce a new ChainType for a new chain, we have to make code changes like this for it anyways.

the idea is that chaintype can deviate from oracle type, e.g if we have the heuristic oracle, many chain types may be able to share the same oracle type

github-actions · 2024-10-04T18:14:15Z

WF: CI Core#8f50535

No errors found in this run. 🎉

core/chains/evm/gas/rollups/op_l1_oracle.go

core/config/docs/chains-evm.toml

core/chains/evm/gas/rollups/op_l1_oracle.go

amit-momin · 2024-10-04T22:44:08Z

Think we'll also need to update the default tomls for chains that use the the OP oracle. Should be the ones with the ChainType's: optimismBedrock, mantle, zircuit, kroma, scroll

docs/CONFIG.md

matYang · 2024-10-07T18:31:25Z

core/chains/evm/gas/rollups/l1_oracle.go

@@ -47,21 +49,21 @@ func IsRollupWithL1Support(chainType chaintype.ChainType) bool {
 	return slices.Contains(supportedChainTypes, chainType)
 }

-func NewL1GasOracle(lggr logger.Logger, ethClient l1OracleClient, chainType chaintype.ChainType) (L1Oracle, error) {
+func NewL1GasOracle(lggr logger.Logger, ethClient l1OracleClient, chainType chaintype.ChainType, daOracle evmconfig.DAOracle) (L1Oracle, error) {


as discussed, we can keep the hardcoded switch statement when daOracle does not exist for backwards-compatibility

…ept-generic-op-l-2-config

cl-sonarqube-production · 2024-10-10T23:43:24Z

Quality Gate passed

Issues
1 New issue
1 Fixed issue
0 Accepted issues

Measures
0 Security Hotspots
80.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

winder · 2024-10-11T18:37:05Z

core/capabilities/ccip/ocrimpls/contract_transmitter_test.go

+func (d *TestDAOracleConfig) OracleAddress() *types.EIP55Address {
+	a, err := types.NewEIP55Address("0x420000000000000000000000000000000000000F")
+	if err != nil {
+		panic(err)
+	}
+	return &a
+}


This looks eth specific but the capability generally tries to keep all the chain specific things separated. Might be worth discussing /w @asoliman92 at some point

matYang reviewed Sep 28, 2024

View reviewed changes

core/chains/evm/config/config.go Show resolved Hide resolved

matYang reviewed Sep 28, 2024

View reviewed changes

core/chains/evm/config/config.go Outdated Show resolved Hide resolved

matYang reviewed Sep 28, 2024

View reviewed changes

core/chains/evm/config/config.go Outdated Show resolved Hide resolved

ogtownsend force-pushed the CCIP-3552-refactor-op-stack-oracle-to-accept-generic-op-l-2-config branch 6 times, most recently from 4fc823a to f158de6 Compare October 2, 2024 17:36

ogtownsend changed the title ~~wip~~ CCIP-3552 add DA oracle config to gas estimator Oct 2, 2024

ogtownsend changed the title ~~CCIP-3552 add DA oracle config to gas estimator~~ CCIP-3552 refactor OP oracle to accept generic DA oracle config Oct 2, 2024

ogtownsend force-pushed the CCIP-3552-refactor-op-stack-oracle-to-accept-generic-op-l-2-config branch 2 times, most recently from 948d154 to 2c95e64 Compare October 3, 2024 00:52

ogtownsend commented Oct 3, 2024

View reviewed changes

core/chains/evm/config/toml/config.go Outdated Show resolved Hide resolved

ogtownsend marked this pull request as ready for review October 3, 2024 03:39

ogtownsend requested review from a team as code owners October 3, 2024 03:39

ogtownsend requested a review from patrickhuie19 October 3, 2024 03:39

ogtownsend force-pushed the CCIP-3552-refactor-op-stack-oracle-to-accept-generic-op-l-2-config branch from b298388 to 7968c5b Compare October 4, 2024 17:56

ogtownsend commented Oct 4, 2024

View reviewed changes

amit-momin reviewed Oct 4, 2024

View reviewed changes

core/chains/evm/gas/rollups/op_l1_oracle.go Outdated Show resolved Hide resolved

amit-momin reviewed Oct 4, 2024

View reviewed changes

core/config/docs/chains-evm.toml Outdated Show resolved Hide resolved

amit-momin reviewed Oct 4, 2024

View reviewed changes

core/chains/evm/gas/rollups/op_l1_oracle.go Outdated Show resolved Hide resolved

ogtownsend force-pushed the CCIP-3552-refactor-op-stack-oracle-to-accept-generic-op-l-2-config branch from 7968c5b to 3c74ccc Compare October 7, 2024 13:46

matYang reviewed Oct 7, 2024

View reviewed changes

matYang requested a review from simsonraj October 7, 2024 20:27

ogtownsend force-pushed the CCIP-3552-refactor-op-stack-oracle-to-accept-generic-op-l-2-config branch from 3c74ccc to 4c7d92b Compare October 7, 2024 20:49

ogtownsend added 12 commits October 10, 2024 13:32

fix tests and some more refactoring

7bf53a2

add tests for custom gas API

7e18916

Rebase

e5623f4

feedback

9b2f749

Fall back to check chainType when creating the L1 oracle

c50b940

update default tomls with DA config

6624bcb

renamings

258830e

remove custom calldata option from op oracle

1f978bd

update namings again

b3fdfd1

change OP oracle type to opstack

986a33b

feedback

226fe74

use random address for tests

6ab6540

ogtownsend dismissed huangzhen1997’s stale review via 6ab6540 October 10, 2024 20:36

ogtownsend force-pushed the CCIP-3552-refactor-op-stack-oracle-to-accept-generic-op-l-2-config branch from e33bf1e to 6ab6540 Compare October 10, 2024 20:36

amit-momin approved these changes Oct 10, 2024

View reviewed changes

patrickhuie19 approved these changes Oct 10, 2024

View reviewed changes

Merge branch 'develop' into CCIP-3552-refactor-op-stack-oracle-to-acc…

8f50535

…ept-generic-op-l-2-config

ogtownsend enabled auto-merge October 10, 2024 23:12

matYang approved these changes Oct 11, 2024

View reviewed changes

winder reviewed Oct 11, 2024

View reviewed changes

winder approved these changes Oct 11, 2024

View reviewed changes

ogtownsend added this pull request to the merge queue Oct 11, 2024

Merged via the queue into develop with commit 4ac405a Oct 11, 2024
149 checks passed

ogtownsend deleted the CCIP-3552-refactor-op-stack-oracle-to-accept-generic-op-l-2-config branch October 11, 2024 19:07

github-actions bot mentioned this pull request Oct 11, 2024

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

Draft

This was referenced Oct 21, 2024

[DO NOT MERGE] Changeset Release Preview - v2.19.0 philipjonsen/chainlink#2

Draft

[DO NOT MERGE] Changeset Release Preview - v2.18.0 picoinnetwork/chainlink#1

Draft

[DO NOT MERGE] Changeset Release Preview - v2.18.0 fanligroup/chainlink#1

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CCIP-3552 refactor OP oracle to accept generic DA oracle config #14599

CCIP-3552 refactor OP oracle to accept generic DA oracle config #14599

ogtownsend commented Sep 27, 2024 •

edited

Loading

ogtownsend Oct 4, 2024

amit-momin Oct 4, 2024

matYang Oct 7, 2024

github-actions bot commented Oct 4, 2024 •

edited

Loading

amit-momin commented Oct 4, 2024

matYang Oct 7, 2024

cl-sonarqube-production bot commented Oct 10, 2024

winder Oct 11, 2024

CCIP-3552 refactor OP oracle to accept generic DA oracle config #14599

CCIP-3552 refactor OP oracle to accept generic DA oracle config #14599

Conversation

ogtownsend commented Sep 27, 2024 • edited Loading

ogtownsend Oct 4, 2024

Choose a reason for hiding this comment

amit-momin Oct 4, 2024

Choose a reason for hiding this comment

matYang Oct 7, 2024

Choose a reason for hiding this comment

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

WF: CI Core#8f50535

amit-momin commented Oct 4, 2024

matYang Oct 7, 2024

Choose a reason for hiding this comment

cl-sonarqube-production bot commented Oct 10, 2024

Quality Gate passed

winder Oct 11, 2024

Choose a reason for hiding this comment

ogtownsend commented Sep 27, 2024 •

edited

Loading

github-actions bot commented Oct 4, 2024 •

edited

Loading