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.

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

BCF 2440 remove chainsets from core #10349

Merged
merged 48 commits into from
Aug 30, 2023
Merged

Conversation

krehermann
Copy link
Contributor

@krehermann krehermann commented Aug 24, 2023

The key points to this are:

BCF-2317 hid all the chainset from public API with the core node
BCF-2564 updated all the chain-specific repos to accept a chain instead of a chainset

The main thrust of this change is

  • plumbs the new signatures implemented in BCF-2564, which removes the internal dependency on chainsets
  • removes the chainset implementations

However, there are some contortions needed to accomplish this while maintain compatibility with the existing loop.Relayer interface.

We have to maintain funcs that had been on the ChainSet implementations:

	ChainStatus(ctx context.Context, id string) (ChainStatus, error)
	ChainStatuses(ctx context.Context, offset, limit int) (chains []ChainStatus, count int, err error)

	NodeStatuses(ctx context.Context, offset, limit int, chainIDs ...string) (nodes []NodeStatus, count int, err error)

	SendTx(ctx context.Context, chainID, from, to string, amount *big.Int, balanceCheck bool) error

We do this with foresight about BCF-2441, which is drafted here: smartcontractkit/chainlink-common#158, and proposes to replace these interface funcs with

  GetChainStatus(ctx context.Context) (ChainStatus, error)
	ListNodeStatuses(ctx context.Context, page_size int32, page_token string) (stats []NodeStatus, next_page_token string, err error)
	// choose different name than SendTx to avoid collison during refactor. we can revert to SendTx if needed after core is rid of conflicting funcs
	Transact(ctx context.Context, from, to string, amount *big.Int, balanceCheck bool) error

Therefore, there are two aspects to playing nicely with the loop.Relayer interface. First is maintaining the exist required funcs in the interface. Second is preemptively adding support for the new interface definition that is drafted above.

In order to ensure an easy transition when BFC-2441 lands, we implement the new funcs and in turn implement the old funcs in terms of them. This is quite nice because we can use all of the existing tests to ensure our new interface funcs are correct. For non-evm chains this can be done in one place, in the RelayExt and RelayAdapter: We define the existing interface funcs on this struct and implement them in terms of the chain that they receive during construction.

EVM is a little more complex, which is to be excepted.

Once BCF-2441 is merged, and we will update this repo to remove all the deleted funcs and expose the already-implemented funcs in there place. There are many TODOs marking the places that can and will be simplified.

core/chains/chain_set.go Outdated Show resolved Hide resolved
core/chains/solana/chain_test.go Outdated Show resolved Hide resolved
core/chains/starknet/chain_set.go Outdated Show resolved Hide resolved
core/chains/starknet/config.go Outdated Show resolved Hide resolved
core/services/chainlink/relayer_factory.go Outdated Show resolved Hide resolved
core/services/chainlink/relayer_factory.go Outdated Show resolved Hide resolved
core/services/chainlink/relayer_factory.go Outdated Show resolved Hide resolved
core/services/relay/evm/relayer_extender_test.go Outdated Show resolved Hide resolved
plugins/cmd/chainlink-solana/main.go Outdated Show resolved Hide resolved
plugins/cmd/chainlink-starknet/main.go Outdated Show resolved Hide resolved
core/chains/evm/chain.go Outdated Show resolved Hide resolved
core/chains/evm/chain.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
@krehermann krehermann force-pushed the BCF-2441-update-loop-interface branch from 3efef0a to 6b519a9 Compare August 29, 2023 20:13
@jmank88
Copy link
Contributor

jmank88 commented Aug 30, 2023

nit/ The branch is prefixed as BCF-2441- - will this confuse Jira?

@jmank88 jmank88 closed this Aug 30, 2023
@jmank88 jmank88 reopened this Aug 30, 2023
@krehermann krehermann requested a review from jmank88 August 30, 2023 17:03
@krehermann krehermann force-pushed the BCF-2441-update-loop-interface branch from 4df5db7 to 1153e16 Compare August 30, 2023 17:26
@cl-sonarqube-production
Copy link

SonarQube Quality Gate

Quality Gate failed

Failed condition 69.1% 69.1% Coverage on New Code (is less than 80%)
Failed condition 15.9% 15.89% Duplicated Lines (%) on New Code (is greater than 3%)

See analysis details on SonarQube

@krehermann krehermann enabled auto-merge August 30, 2023 18:17
@krehermann krehermann added this pull request to the merge queue Aug 30, 2023
Merged via the queue into develop with commit b41e626 Aug 30, 2023
@krehermann krehermann deleted the BCF-2441-update-loop-interface branch August 30, 2023 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants