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

Add DiscoveryProcessor to commit plugin. #141

Merged
merged 3 commits into from
Sep 20, 2024
Merged

Conversation

winder
Copy link
Contributor

@winder winder commented Sep 19, 2024

Add contract discovery to the commit plugin.

@winder winder force-pushed the will/onramp-observations-pt5 branch 3 times, most recently from fe0af79 to 4ee5e32 Compare September 19, 2024 18:02
@winder winder marked this pull request as ready for review September 19, 2024 18:44
@winder winder requested a review from a team as a code owner September 19, 2024 18:44
Copy link

Test Coverage

Branch Coverage
will/onramp-observations-pt5 71.7%
main 71.6%

if err != nil {
p.lggr.Errorw("failed to discover contracts", "err", err)
}
if !p.contractsInitialized {
Copy link
Contributor

Choose a reason for hiding this comment

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

If contract addresses change, is there a risk of having stale addresses?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Contract addresses won't change but we could have new chains added while the plugin is running.

However, the discovery processor continuously discovers new addresses at the moment, so these new chains get picked up. There's also a test for this in chainlink.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A new address would be a problem because we don't unbind the old one and currently ensure that there is only one bound address. This is something we could support pretty easily if we need to in the future.

@winder winder merged commit 1b134f6 into main Sep 20, 2024
3 checks passed
@winder winder deleted the will/onramp-observations-pt5 branch September 20, 2024 11:30
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.

3 participants