[NONEVM-904] Update address book to support nonevm #15269

jlaveracll · 2024-11-15T17:11:40Z

Requires

Supports

github-actions · 2024-11-15T17:13:09Z

I see you updated files related to core. Please run pnpm changeset in the root directory to add a changeset as well as in the text include at least one of the following tags:

#added For any new functionality added.
#breaking_change For any functionality that requires manual action for the node to boot.
#bugfix For bug fixes.
#changed For any change to the existing functionality.
#db_update For any feature that introduces updates to database schema.
#deprecation_notice For any upcoming deprecation functionality.
#internal For changesets that need to be excluded from the final changelog.
#nops For any feature that is NOP facing and needs to be in the official Release Notes for the release.
#removed For any functionality/config that is removed.
#updated For any functionality that is updated.
#wip For any change that is not ready yet and external communication about it should be held off till it is feature complete.

🎖️ No JIRA issue number found in: PR title, commit message, or branch name. Please include the issue ID in one of these.

github-actions · 2024-11-15T17:37:20Z

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

connorwstein

A large number of the comments are a little redundant because they are in EVM specific contexts but fine just keep em and over-communicate imo.

connorwstein · 2024-11-19T14:25:52Z

deployment/address_book.go

-		address = common.HexToAddress(address).Hex()
-	} else {
-		return errors.Wrapf(ErrInvalidAddress, "address %s is not a valid Ethereum address, only Ethereum addresses supported", address)
+	if family == chainsel.FamilyEVM {


could add your aptos specific validation?

we don't have any yet, but we'll create a ticket to add it later. We are already tackling hardening tasks like this one

I was toying with an idea of building in aptos and other chain or family validations into the chainlink-selectors repo, so we'd have a set of reusable validations keyed off of chain ID in a way that hides all the conditionality from calling code. Valid addresses was definitely in the right zone. Doing it here is fine, but we should think about making this more generally usable.

connorwstein · 2024-11-19T14:27:25Z

deployment/address_book.go

-	_, exists := chainsel.ChainBySelector(chainSelector)
-	if !exists {
+// Save will save an address for a given chain selector. It will error if there is a conflicting existing address.
+func (m *AddressBookMap) Save(chainSelector uint64, address string, typeAndVersion TypeAndVersion) error {


Let's update the Save test with some other family addresses

why did this become public? i don't see anything that using it in other packages.

case added!

yeah +1 @krehermann this should remain a private helper, there's already a public Save. Not sure how this actually compiles it looks like it has the same signature as the public Save?

…update-addressbook-to-support-nonevm

cgruber · 2024-11-19T20:42:34Z

core/services/relay/evm/evm.go

@@ -197,6 +197,7 @@ func NewRelayer(ctx context.Context, lggr logger.Logger, chain legacyevm.Chain,
 	sugared := logger.Sugared(lggr).Named("Relayer")
 	mercuryORM := mercury.NewORM(opts.DS)
 	cdcFactory := sync.OnceValues(func() (llo.ChannelDefinitionCacheFactory, error) {
+		// NOTE: This does not support non-evm chains


Are these notes intended to be TODOs, or is this purely documentary?

documentation

github-actions · 2024-11-21T15:13:55Z

I see you added a changeset file but it does not contain a tag. Please edit the text include at least one of the following tags:

#added For any new functionality added.
#breaking_change For any functionality that requires manual action for the node to boot.
#bugfix For bug fixes.
#changed For any change to the existing functionality.
#db_update For any feature that introduces updates to database schema.
#deprecation_notice For any upcoming deprecation functionality.
#internal For changesets that need to be excluded from the final changelog.
#nops For any feature that is NOP facing and needs to be in the official Release Notes for the release.
#removed For any functionality/config that is removed.
#updated For any functionality that is updated.
#wip For any change that is not ready yet and external communication about it should be held off till it is feature complete.

…update-addressbook-to-support-nonevm

Update address book to support nonevm

58c3eb6

jlaveracll changed the title ~~Update address book to support nonevm~~ [NONEVM-904] Update address book to support nonevm Nov 15, 2024

Update address book to include nonevm

c10af69

jlaveracll added 2 commits November 19, 2024 10:33

avoid address validation for non evm chains

4132479

merge'

1ad01a2

jlaveracll marked this pull request as ready for review November 19, 2024 13:35

jlaveracll requested review from a team as code owners November 19, 2024 13:35

jlaveracll requested review from winder, rstout, asoliman92, makramkd, dimkouv, mateusz-sekara, 0xAustinWang and krehermann November 19, 2024 13:35

connorwstein reviewed Nov 19, 2024

View reviewed changes

jlaveracll added 2 commits November 19, 2024 15:02

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

9c2398d

…update-addressbook-to-support-nonevm

address feedback

b5b95e1

jmank88 previously approved these changes Nov 19, 2024

View reviewed changes

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

5fc8f07

…update-addressbook-to-support-nonevm

jlaveracll dismissed jmank88’s stale review via 5fc8f07 November 20, 2024 13:47

cgruber previously approved these changes Nov 20, 2024

View reviewed changes

connorwstein previously approved these changes Nov 20, 2024

View reviewed changes

jmank88 previously approved these changes Nov 20, 2024

View reviewed changes

Update plugin.go

122241f

bump chain selector to 1.0.31

f013d31

jlaveracll requested review from a team as code owners November 21, 2024 18:03

merge

0a2ef89

connorwstein previously approved these changes Nov 21, 2024

View reviewed changes

prashantkumar1982 approved these changes Nov 21, 2024

View reviewed changes

prashantkumar1982 previously approved these changes Nov 21, 2024

View reviewed changes

Merge branch 'develop' into update-addressbook-to-support-nonevm

fb24bb7

prashantkumar1982 dismissed stale reviews from connorwstein and themself via fb24bb7 November 21, 2024 22:02

prashantkumar1982 previously approved these changes Nov 21, 2024

View reviewed changes

krehermann previously approved these changes Nov 21, 2024

View reviewed changes

ran make gomodtidy

986a6a4

jlaveracll dismissed stale reviews from krehermann and prashantkumar1982 via 986a6a4 November 22, 2024 04:24

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

3ba1ff2

…update-addressbook-to-support-nonevm

connorwstein approved these changes Nov 22, 2024

View reviewed changes

krehermann approved these changes Nov 22, 2024

View reviewed changes

krehermann added this pull request to the merge queue Nov 22, 2024

krehermann approved these changes Nov 22, 2024

View reviewed changes

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

prashantkumar1982 approved these changes Nov 22, 2024

View reviewed changes

prashantkumar1982 added this pull request to the merge queue Nov 22, 2024

Merged via the queue into develop with commit 00777b8 Nov 22, 2024
148 of 149 checks passed

prashantkumar1982 deleted the update-addressbook-to-support-nonevm branch November 22, 2024 18:41

This was referenced Nov 22, 2024

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

Draft

[DO NOT MERGE] Changeset Release Preview - v2.19.0 galactic-beyond/chainlink#1

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NONEVM-904] Update address book to support nonevm #15269

[NONEVM-904] Update address book to support nonevm #15269

jlaveracll commented Nov 15, 2024

github-actions bot commented Nov 15, 2024

github-actions bot commented Nov 15, 2024 •

edited

Loading

connorwstein left a comment

connorwstein Nov 19, 2024

jlaveracll Nov 19, 2024

cgruber Nov 19, 2024

connorwstein Nov 19, 2024

krehermann Nov 19, 2024

jlaveracll Nov 19, 2024

connorwstein Nov 19, 2024

cgruber Nov 19, 2024

jlaveracll Nov 20, 2024

github-actions bot commented Nov 21, 2024

[NONEVM-904] Update address book to support nonevm #15269

[NONEVM-904] Update address book to support nonevm #15269

Conversation

jlaveracll commented Nov 15, 2024

Requires

Supports

github-actions bot commented Nov 15, 2024

github-actions bot commented Nov 15, 2024 • edited Loading

AER Report: CI Core ran successfully ✅

AER Report: Operator UI CI ran successfully ✅

connorwstein left a 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

github-actions bot commented Nov 21, 2024

github-actions bot commented Nov 15, 2024 •

edited

Loading