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

Specify the number of sidecar subnets #3346

Merged
merged 2 commits into from
May 18, 2023
Merged

Specify the number of sidecar subnets #3346

merged 2 commits into from
May 18, 2023

Conversation

ppopth
Copy link
Member

@ppopth ppopth commented May 5, 2023 •
edited
Loading

Previously the number of subnets is equal to MAX_BLOBS_PER_BLOCK which specifies the number of blobs per block. This commit now makes the number of subnets equal to BLOB_SIDECAR_SUBNET_COUNT instead.

The advantage of doing this is that we can change MAX_BLOBS_PER_BLOCK without worrying about the p2p network structure and the number of subnets.

@ppopth ppopth mentioned this pull request May 5, 2023
Merged
@djrtwo djrtwo closed this May 8, 2023
@djrtwo djrtwo reopened this May 8, 2023
djrtwo
djrtwo approved these changes May 8, 2023
View reviewed changes
Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

looks good!

I'm not immediately sure why test-eip-6110 CI failed. I just re-triggeredd the build to see if that helps

hwwhww
hwwhww approved these changes May 9, 2023
View reviewed changes
specs/deneb/validator.md Outdated Show resolved Hide resolved
@ppopth ppopth force-pushed the sidecar-subnets branch 2 times, most recently from eef0b0c to a8c3891 Compare May 10, 2023 15:36
@ppopth
Copy link
Member Author

ppopth commented May 15, 2023

@hwwhww Do you have an insight on the CI error?

python3 -m pytest -n 16 --bls-type=fastest --preset=minimal --fork=eip6110 --junitxml=test-reports/test_results.xml eth2spec
/home/circleci/specs-repo/venv/bin/python3: No module named pytest
make: *** [Makefile:123: citest] Error 1

@ppopth ppopth mentioned this pull request May 15, 2023
Closed
@hwwhww
Copy link
Contributor

hwwhww commented May 16, 2023

@ppopth I believe since EIP6110 CI job was merged to the CI dev branch after you opened this PR, the cache was weird and broken. We can just ignore it here.

@timbeiko timbeiko mentioned this pull request May 16, 2023
Closed
ralexstokes
ralexstokes approved these changes May 17, 2023
View reviewed changes
Copy link
Member

@ralexstokes ralexstokes left a comment

Choose a reason for hiding this comment

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

good idea

g11tech
g11tech approved these changes May 18, 2023
View reviewed changes
Copy link
Contributor

@g11tech g11tech left a comment

Choose a reason for hiding this comment

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

lgtm!

@ppopth
Specify the number of sidecar subnets
2f218f8 
Previously the number of subnets is equal to MAX_BLOBS_PER_BLOCK which
specifies the number of blobs per block. This commit now makes the
number of subnets equal to BLOB_SIDECAR_SUBNET_COUNT instead.

The advantage of doing this is that we can change MAX_BLOBS_PER_BLOCK
without worrying about the p2p network structure and the number of subnets.
@ppopth
Use SubnetID instead of uint64
08a1326
@djrtwo djrtwo merged commit a1c33c9 into ethereum:dev May 18, 2023
@ppopth ppopth deleted the sidecar-subnets branch May 18, 2023 18:17
@ralexstokes ralexstokes mentioned this pull request May 18, 2023
Merged
@StefanBratanov StefanBratanov mentioned this pull request May 23, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@djrtwo djrtwo djrtwo approved these changes

@ralexstokes ralexstokes ralexstokes approved these changes

@hwwhww hwwhww hwwhww approved these changes

@g11tech g11tech g11tech approved these changes

Assignees
No one assigned
Labels
scope:networking scope:v-guide Validator guide
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ppopth @hwwhww @djrtwo @ralexstokes @g11tech