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

update common repo; apply patch of e9e903bf4b34099f8b274eb1e0f013b4ab… #684

Open
wants to merge 8 commits into
base: ccip-develop
Choose a base branch
from

Conversation

krehermann
Copy link
Collaborator

@krehermann krehermann commented Apr 4, 2024

Motivation

Update the dependencies so new features can be developed in chainlink-common. This means reconciling the interfaces in this repo with the updated definitions in common. The drift was ctx and err handling.

Solution

The repo was behind a number of change in core & common. It was impossible to only bump common and then tweak ccip code.

Instead I found all the changes in common that have propagated to core and required here to compile, created a patch for each of them, and applied the patches. This was straightforward but annoying.

Beyond plumbing ctx and error there was one significant change in the interfaces -- Close'ing and how that is intertwined with logpoller filters.

gRPC can't understand pg.Opts. It is not possible to maintain the existing Close signature, which was a thin wrapper around logpoller.UnregisterFilter(). However, it is required that Close exist so that resources can be freed on the server.

The simplest path forward was to extract the existing behavior of Close into a standalone func UnregisterFilters. Given that Close signature changed, it's easy to ensure that all the call sites are correctly modified to call UnregisterFilters in addition to Close.

This solution is not pretty, but it is sufficient, and pg.Opts has to be removed in the near future to be compatible with EVM extraction.

@krehermann krehermann marked this pull request as ready for review April 5, 2024 01:08
@krehermann krehermann requested review from RensR, dimkouv, makramkd and a team as code owners April 5, 2024 01:08
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.

None yet

2 participants