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

client: add getBlobsV1 to the client to support CL blob import #3711

Merged
merged 15 commits into from
Oct 7, 2024

Conversation

g11tech
Copy link
Contributor

@g11tech g11tech commented Sep 29, 2024

Copy link
Member

@jochem-brouwer jochem-brouwer left a comment

Choose a reason for hiding this comment

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

Strong support for the API since we need to bring bandwidth requirements of a running a node down 😄

Two comments.

packages/client/src/service/txpool.ts Show resolved Hide resolved
packages/client/src/service/txpool.ts Outdated Show resolved Hide resolved
Copy link

codecov bot commented Oct 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 31.22%. Comparing base (4470cc3) to head (a6d5c58).
Report is 86 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 67.57% <ø> (-5.89%) ⬇️
blockchain 83.49% <ø> (?)
client ?
common 89.85% <ø> (?)
genesis 0.00% <ø> (?)
tx ?
util 70.93% <ø> (?)
wallet ?

Flags with carried forward coverage won't be shown. Click here to find out more.

@jochem-brouwer
Copy link
Member

The spec says:

The fields are encoded as follows:

- `blob`: `DATA` - `FIELD_ELEMENTS_PER_BLOB * BYTES_PER_FIELD_ELEMENT = 4096 * 32 = 131072` bytes (`DATA`) representing a SSZ-encoded `Blob` as defined in [EIP-4844](https://eips.ethereum.org/EIPS/eip-4844).

It does however not look like we SSZ-encode the blob? I think this is mandatory here? (per spec)

@g11tech
Copy link
Contributor Author

g11tech commented Oct 2, 2024

The spec says:

The fields are encoded as follows:

- `blob`: `DATA` - `FIELD_ELEMENTS_PER_BLOB * BYTES_PER_FIELD_ELEMENT = 4096 * 32 = 131072` bytes (`DATA`) representing a SSZ-encoded `Blob` as defined in [EIP-4844](https://eips.ethereum.org/EIPS/eip-4844).

It does however not look like we SSZ-encode the blob? I think this is mandatory here? (per spec)

this the blobs generated/provided in tx are already in ssz encoded format which is just 131072 bytes, we don't have to do anything here

1 similar comment
@g11tech
Copy link
Contributor Author

g11tech commented Oct 2, 2024

The spec says:

The fields are encoded as follows:

- `blob`: `DATA` - `FIELD_ELEMENTS_PER_BLOB * BYTES_PER_FIELD_ELEMENT = 4096 * 32 = 131072` bytes (`DATA`) representing a SSZ-encoded `Blob` as defined in [EIP-4844](https://eips.ethereum.org/EIPS/eip-4844).

It does however not look like we SSZ-encode the blob? I think this is mandatory here? (per spec)

this the blobs generated/provided in tx are already in ssz encoded format which is just 131072 bytes, we don't have to do anything here

@jochem-brouwer
Copy link
Member

Ok, I was not aware that the blobs were already SSZ-encoded (do not see this in EIP-4844), but if that's the case that's great, otherwise we implicitly have to take in the SSZ dependency (which we will have to do at some point when we switch to SSZ from RLP in general)

Copy link
Member

@jochem-brouwer jochem-brouwer left a comment

Choose a reason for hiding this comment

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

LGTM!

@g11tech g11tech merged commit 27b8e9f into master Oct 7, 2024
39 checks passed
@holgerd77 holgerd77 deleted the getblobsv1 branch October 7, 2024 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants