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

Fix the potentially inconsistent issue. #14700

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

arcane4096
Copy link

@arcane4096 arcane4096 commented Dec 8, 2024

What type of PR is this?
Improvements

What does this PR do? Why is it needed?
If the endpoint parameter of the execution layer specifies a domain name, for example: --execution-endpoint=http://execution-domain:8551, and the execution layer clients corresponding to the domain name include Geth and Reth, then when Reth client is accessed first and the list of capabilities obtained contains engine_getBlobsV1. However, when the connection is reconnected to Geth client, engine_getBlobsV1 is not removed from capabilityCache, which may lead to unexpected results.

Acknowledgements

  • I have read CONTRIBUTING.md.
  • I have made an appropriate entry to CHANGELOG.md.
  • I have added a description to this PR with sufficient context for reviewers to understand this PR.

@arcane4096 arcane4096 requested a review from a team as a code owner December 8, 2024 13:43
@CLAassistant
Copy link

CLAassistant commented Dec 8, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@terencechain terencechain left a comment

Choose a reason for hiding this comment

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

I think this makes sense. I wonder how it would work when a single beacon node is driving multiple execution layer clients:

  • One Prysm instance drives both Geth and Reth. Geth supports get_blobs, but Reth does not
  • Prysm connects to Geth first, then connects to Reth
  • Prysm will not use get_blobs even though it could

@arcane4096
Copy link
Author

Hi, From the code, it can be seen that after requesting engine_exchangeCapabilities and obtaining results, the save function is called, and the cache does not clear the previous old cache data.

If Reth returns [engine_getBlobsV1, OTHERS], but when requesting Geth, it returns [OTHERS], and since the cache is not cleared, calling cache.has("engine_getBlobsV1") again will return true, and the subsequent logic will be executed (which may not have a significant impact).

@arcane4096
Copy link
Author

Hi @terencechain A more likely scenario is that Geth runs on a version 1.14.12 for a while and then reverts to a version below 1.14.12 .

Geth supports engine_getBlobsV1 starting from version 1.14.12

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