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.

Sign up for GitHub

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

Jump to bottom

[BCFR-203] Improve CR value comparator querying (topics and data words) by doing encoding in the relayer #14207

Merged
merged 28 commits into from
Sep 18, 2024
Merged

[BCFR-203] Improve CR value comparator querying (topics and data words) by doing encoding in the relayer #14207

merged 28 commits into from
Sep 18, 2024

Conversation

ilija42
Copy link
Contributor

@ilija42 ilija42 commented Aug 22, 2024 •
edited
Loading

Description

  • Implement QueryKey Value Comparator filters encoding. This enables the caller to just provide a raw value for filtering which then gets encoded and modified in the relayer to match the on chain version. For eg. chain agnostic code had to hash a string value like solc to filter over topic values, but now can just provide a raw string value which will be encoded in the EVM Chain Reader impl. This works for filtering over indexed topics and simpler cases of non-indexed topics aka data words where offsets don't have to be calcualted.

Requires Dependencies

common

@ilija42 ilija42 force-pushed the BCI-3940-Improve-CR-Querying branch from 4fc5e96 to e15e8a8 Compare August 29, 2024 19:22
@ilija42 ilija42 changed the title Bci 3940 improve cr querying [BCI - 3940] improve CR value comparator querying (topics and data words) by doing encoding in the relayer Aug 30, 2024
@ilija42 ilija42 changed the title [BCI - 3940] improve CR value comparator querying (topics and data words) by doing encoding in the relayer [BCI - 3940] Improve CR value comparator querying (topics and data words) by doing encoding in the relayer Aug 30, 2024
@ilija42 ilija42 mentioned this pull request Sep 2, 2024
Merged
@ilija42 ilija42 marked this pull request as ready for review September 2, 2024 15:46
@ilija42 ilija42 requested review from jmank88 and a team and removed request for a team September 2, 2024 15:46
EasterTheBunny
EasterTheBunny previously approved these changes Sep 3, 2024
View reviewed changes
@ilija42 ilija42 force-pushed the BCI-3940-Improve-CR-Querying branch 2 times, most recently from 3b9cacb to 5c682a9 Compare September 11, 2024 13:03
@ilija42 ilija42 force-pushed the BCI-3940-Improve-CR-Querying branch 5 times, most recently from d6da4c2 to 8825a91 Compare September 16, 2024 12:51
@ilija42 ilija42 changed the title [BCI - 3940] Improve CR value comparator querying (topics and data words) by doing encoding in the relayer [BCFR-203] Improve CR value comparator querying (topics and data words) by doing encoding in the relayer Sep 16, 2024
@ilija42 ilija42 force-pushed the BCI-3940-Improve-CR-Querying branch from 8825a91 to 0f37797 Compare September 16, 2024 13:02
ilija42
ilija42 commented Sep 16, 2024
View reviewed changes
core/services/relay/evm/chain_reader.go

// TODO can this be simplified? Isn't this same as inputInfo.Modifier()? BCI-3909
inMod, err := def.InputModifications.ToModifier(codec.DecoderHooks...)
dWsDetail[genericName] = dWDetail
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just realised that this PR ran over this PR, so this TODO is getting re-resolved

@ilija42 ilija42 force-pushed the BCI-3940-Improve-CR-Querying branch from cf2059b to fb18665 Compare September 18, 2024 16:07
Farber98
Farber98 previously approved these changes Sep 18, 2024
View reviewed changes
EasterTheBunny
EasterTheBunny previously approved these changes Sep 18, 2024
View reviewed changes
silaslenihan
silaslenihan previously approved these changes Sep 18, 2024
View reviewed changes
@ilija42 ilija42 enabled auto-merge September 18, 2024 21:09
@cl-sonarqube-production
Copy link

Quality Gate passed Quality Gate passed

Issues
5 New issues
2 Fixed issues
0 Accepted issues

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

See analysis details on SonarQube

@ilija42 ilija42 added this pull request to the merge queue Sep 18, 2024
Merged via the queue into develop with commit 328b62a Sep 18, 2024
164 of 165 checks passed
@ilija42 ilija42 deleted the BCI-3940-Improve-CR-Querying branch September 18, 2024 22:06
@github-actions github-actions bot mentioned this pull request Sep 18, 2024
Draft
momentmaker added a commit that referenced this pull request Sep 19, 2024
@momentmaker
Merge branch 'develop' into re-2964/goreleaser-ccip
15cbe34 
* develop: (79 commits)
  core/config/toml: include value in InsecureConnection error message (#14495)
  vrf: add soneium config for integration tests (#14443)
  Add default test config for Grafana (#14475)
  Update to 1.5 RMN contract address (#14485)
  fix lint issues (#14450)
  [BCFR-203] Improve CR value comparator querying (topics and data words) by doing encoding in the relayer (#14207)
  integration-tests/deployment/ccip: fix assertion fns (#14482)
  chore: remove gha workflow validation (#14487)
  devsvcs-244: update token transfer (#14481)
  DF-20481: Add new OCR3DataFeeds telemetry type for Mercury jobs (#14470)
  Merc 6304 view function ea telem support (#14467)
  [DEVSVCS-545] cleanup (#14478)
  Mantle use vanilla l1 oracle (#14471)
  Fix E2E_TESTS_ON_GITHUB_CI.md (#14473)
  Upgrade chainlink ccip dependency (#14474)
  [DEVSVCS-545] automation benchmark test remove support for keepers v1 (#14472)
  Enables OOO Execution for E2E Tests (#14392)
  Delegate workflow spec creation (#14365)
  CCIP-3407: Add release testing configs (#1436) (#14463)
  Implement v0.3-compatible telemetry for LLO (#14440)
  ...
@github-actions github-actions bot mentioned this pull request Sep 20, 2024
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmank88 jmank88 jmank88 approved these changes

@silaslenihan silaslenihan silaslenihan approved these changes

@RensR RensR Awaiting requested review from RensR

@matYang matYang Awaiting requested review from matYang

@RayXpub RayXpub Awaiting requested review from RayXpub

@elatoskinas elatoskinas Awaiting requested review from elatoskinas

@EasterTheBunny EasterTheBunny Awaiting requested review from EasterTheBunny

@Farber98 Farber98 Awaiting requested review from Farber98

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ilija42 @jmank88 @EasterTheBunny @silaslenihan @Farber98