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

Grandine CL < > Lodestar VC incompatibility #6637

Closed
barnabasbusa opened this issue Apr 5, 2024 · 16 comments
Closed

Grandine CL < > Lodestar VC incompatibility #6637

barnabasbusa opened this issue Apr 5, 2024 · 16 comments
Labels
meta-bug Issues that identify a bug and require a fix. scope-interop Issues that fix interop issues between Lodestar and CL, EL or tooling.

Comments

@barnabasbusa
Copy link
Contributor

barnabasbusa commented Apr 5, 2024

Describe the bug

We are in the process to test cross beacon <> validator client compatibility, and found a bug when testing Grandine CL with lodestar VC.

Lodestar reports:

Apr-05 11:27:04.125[]                error: Error proposing block slot=1, validator=0xa158…501f - Failed to produce block: validator.produceBlockV3 - Bad Request: invalid query string: unknown field `fee_recipient`, expected one of `randao_reveal`, `graffiti`, `skip_randao_verification` - Failed to produce block
Error: Failed to produce block: validator.produceBlockV3 - Bad Request: invalid query string: unknown field `fee_recipient`, expected one of `randao_reveal`, `graffiti`, `skip_randao_verification` - Failed to produce block
    at Function.assert (file:///usr/app/packages/api/src/utils/client/httpClient.ts:44:13)
    at BlockProposingService.produceBlockWrapper (file:///usr/app/packages/validator/src/services/block.ts:212:14)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at BlockProposingService.createAndPublishBlock (file:///usr/app/packages/validator/src/services/block.ts:144:29)
    at async Promise.all (index 0)

Grandine reports:

nothing out of the ordinary

Snooper between cl <> vc reports:

nothing out of the ordinary

cc: @pk910

Expected behavior

I would expect all client combinations would work.

Steps to reproduce

config.yaml:

participants:
  - cl_type: grandine
    vc_type: lodestar
additional_services:
  - dora
snooper_enabled: true
global_log_level: debug

kurtosis run github.com/kurtosis-tech/ethereum-package --args-file config.yaml

Additional context

Current BN <> VC Compatibility list tracker

Screenshot 2024-04-05 at 14 08 53

Operating system

Linux

Lodestar version or commit hash

Version: v1.17.0/898cd90

@barnabasbusa barnabasbusa added the meta-bug Issues that identify a bug and require a fix. label Apr 5, 2024
@barnabasbusa
Copy link
Contributor Author

Cross reference of: grandinetech/grandine#24

@sauliusgrigaitis
Copy link

Any chance for Lodestar to not send this field by default? (Maybe you can send it if CLI flag is passed)

@nflaig
Copy link
Member

nflaig commented Apr 5, 2024

Any chance for Lodestar to not send this field by default? (Maybe you can send it if CLI flag is passed)

This is definitely an issue on our end, we were sending this query param always with a boolean value, e.g. by default skip_randao_verification=false. We have fixed this recently #6576 but it's not yet released.

@nflaig nflaig added the scope-interop Issues that fix interop issues between Lodestar and CL, EL or tooling. label Apr 5, 2024
@sauliusgrigaitis
Copy link

sauliusgrigaitis commented Apr 5, 2024

@nflaig I just noticed that @barnabasbusa mixed errors Discord.

The problem is actually fee_recipient field:

Apr-05 11:27:04.125[]                error: Error proposing block slot=1, validator=0xa158…501f - Failed to produce block: validator.produceBlockV3 - Bad Request: invalid query string: unknown field `fee_recipient`, expected one of `randao_reveal`, `graffiti`, `skip_randao_verification` - Failed to produce block
Error: Failed to produce block: validator.produceBlockV3 - Bad Request: invalid query string: unknown field `fee_recipient`, expected one of `randao_reveal`, `graffiti`, `skip_randao_verification` - Failed to produce block
    at Function.assert (file:///usr/app/packages/api/src/utils/client/httpClient.ts:44:13)
    at BlockProposingService.produceBlockWrapper (file:///usr/app/packages/validator/src/services/block.ts:212:14)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at BlockProposingService.createAndPublishBlock (file:///usr/app/packages/validator/src/services/block.ts:144:29)
    at async Promise.all (index 0)

@barnabasbusa
Copy link
Contributor Author

@nflaig indeed, I mixed up the issues with nimbus and grandine. Please see the correct error message above.

@sauliusgrigaitis good catch!

@nflaig
Copy link
Member

nflaig commented Apr 5, 2024

I am not sure we can resolve this on the Lodestar side as we have a bunch of query params that are not part of the spec but Lodestar itself uses.

skip_randao_verification?: string;
fee_recipient?: string;
builder_selection?: string;
builder_boost_factor?: string;
strict_fee_recipient_check?: boolean;
blinded_local?: boolean;

It seems like all other clients just ignore extraneous query params and we should probably avoid implementing client specific workarounds.

But let's ask @g11tech who added most / all of those extra params.

@sauliusgrigaitis
Copy link

@nflaig @g11tech is it the only request that you send unspec'ed parameters?

@nflaig
Copy link
Member

nflaig commented Apr 5, 2024

@nflaig @g11tech is it the only request that you send unspec'ed parameters?

Yes, I think this is the only api. Also since we allow to produce a blinded local block, we send execution_payload_source as part of the response body, this has been proposed in ethereum/beacon-APIs#387 but did not make it into block v3 api.

@barnabasbusa
Copy link
Contributor Author

barnabasbusa commented Apr 5, 2024

Trying again with grandine's latest image (which ignores extra fields)
ethpandaops/grandine:ignore-extra-fields-c28d828

Apr-05 14:33:42.048[]                error: Error proposing block slot=72, validator=0x8419…5932 - Failed to produce block: validator.produceBlockV3 - Internal Server Error: {
  "id": 1,
  "jsonrpc": "2.0",
  "error": {
    "code": -32603,
    "message": "Error processing response: encountered hyper error: hyper::Error(IncompleteMessage)"
  }
} - Failed to produce block
Error: Failed to produce block: validator.produceBlockV3 - Internal Server Error: {
  "id": 1,
  "jsonrpc": "2.0",
  "error": {
    "code": -32603,
    "message": "Error processing response: encountered hyper error: hyper::Error(IncompleteMessage)"
  }
} - Failed to produce block
    at Function.assert (file:///usr/app/packages/api/src/utils/client/httpClient.ts:44:13)
    at BlockProposingService.produceBlockWrapper (file:///usr/app/packages/validator/src/services/block.ts:212:14)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at BlockProposingService.createAndPublishBlock (file:///usr/app/packages/validator/src/services/block.ts:144:29)
    at async Promise.all (index 0)
    ```

@nflaig
Copy link
Member

nflaig commented Apr 5, 2024

Trying again with grandine's latest image (which ignores extra fields) ethpandaops/grandine:ignore-extra-fields-c28d828

Looks like an internal error in Grandine

@sauliusgrigaitis
Copy link

@barnabasbusa @nflaig It's skip_randao_verification=false now. @barnabasbusa maybe you can build Lodestar image with tha t fix #6576 ?

@nflaig
Copy link
Member

nflaig commented Apr 5, 2024

@barnabasbusa @nflaig It's skip_randao_verification=false now. @barnabasbusa maybe you can build Lodestar image with tha t fix #6576 ?

You can just use chainsafe/lodestar:next which is build from our unstable branch on every commit

@g11tech
Copy link
Contributor

g11tech commented Apr 8, 2024

i would request grandine @sauliusgrigaitis to ignore the non spec params, they are essentially there because we need them for one purpose or another.

their defaults are spec complaint (so they assume values in accordance with spec) and responses are modified only if they were requested by these non spec aware valdiators/middlewares (DVT)

and extra response flags carry the information if they are applied as well with again defaults of those flags in spec complaint manner which again a non aware validator software for e.g. can safely ignore.

@pk910
Copy link

pk910 commented Apr 8, 2024

This is fixed in latest grandine develop version :)
Grandine is now ignoring these additional parameters, so probably nothing to do from lodestar side.

@sauliusgrigaitis
Copy link

We ignore not spec'ed parameters in in particular places to achieve compatibility. However, we will probably ignore it all requests later. So this can be closed I think.

@nflaig
Copy link
Member

nflaig commented Apr 9, 2024

We ignore not spec'ed parameters in in particular places to achieve compatibility. However, we will probably ignore it all requests later. So this can be closed I think.

Great, thanks @sauliusgrigaitis, I also think this is the best behavior as it gives clients the opportunity to experiment with new configurations even if those are not part of the spec yet.

Thanks for retesting @pk910 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta-bug Issues that identify a bug and require a fix. scope-interop Issues that fix interop issues between Lodestar and CL, EL or tooling.
Projects
None yet
Development

No branches or pull requests

5 participants