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

Request Data Columns When Fetching Pending Blocks #14007

Merged
merged 10 commits into from
May 17, 2024

Conversation

nisdas
Copy link
Member

@nisdas nisdas commented May 16, 2024

What type of PR is this?

Feature

What does this PR do? Why is it needed?

We support fetching data columns when trying to process pending blocks, in order to be able to process these blocks we need to fetch the desired custody columns.

  • Support filtering peers by those which custody our desired columns
  • Allow pending blocks with columns to be correctly processed.

Which issues(s) does this PR fix?

N.A

Other notes for review

@nisdas nisdas added Ready For Review A pull request ready for code review peerDAS labels May 16, 2024
@nisdas nisdas requested a review from a team as a code owner May 16, 2024 09:51
@nisdas nisdas requested review from potuz, rkapka and james-prysm and removed request for a team May 16, 2024 09:51
@nisdas nisdas changed the title Request Data Columns When Fetching Pending Block By Root Request Request Data Columns When Fetching Pending Blocks May 16, 2024
@nisdas nisdas requested a review from prestonvanloon as a code owner May 16, 2024 14:03
@nisdas nisdas force-pushed the supportDataColumnsInByRootRequests branch from a6e68e9 to 535568f Compare May 16, 2024 14:35
@nalepae nalepae force-pushed the supportDataColumnsInByRootRequests branch from 535568f to df9d7bc Compare May 17, 2024 08:11
Comment on lines 211 to 247
var request p2ptypes.BlobSidecarsByRootReq
var err error
if features.Get().EnablePeerDAS {
request, err = s.pendingDataColumnRequestForBlock(blkRoot, b)
if err != nil {
return err
}
if err := s.sendAndSaveBlobSidecars(ctx, request, peers[rand.NewGenerator().Int()%peerCount], b); err != nil {
} else {
request, err = s.pendingBlobsRequestForBlock(blkRoot, b)
if err != nil {
return err
}
}
if features.Get().EnablePeerDAS {
if len(request) > 0 {
peers := s.getBestPeers()
peers, err = s.getValidCustodyPeers(peers)
if err != nil {
return err
}
peerCount := len(peers)
if peerCount == 0 {
return errors.Wrapf(errNoPeersForPending, "block root=%#x", blkRoot)
}
if err := s.sendAndSaveDataColumnSidecars(ctx, request, peers[rand.NewGenerator().Int()%peerCount], b); err != nil {
return err
}
}
} else {
if len(request) > 0 {
peers := s.getBestPeers()
peerCount := len(peers)
if peerCount == 0 {
return errors.Wrapf(errNoPeersForPending, "block root=%#x", blkRoot)
}
if err := s.sendAndSaveBlobSidecars(ctx, request, peers[rand.NewGenerator().Int()%peerCount], b); err != nil {
return err
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be replaced by:

	if features.Get().EnablePeerDAS {
		request, err := s.pendingDataColumnRequestForBlock(blkRoot, b)
		if err != nil {
			return errors.Wrap(err, "pending data column request for block")
		}

		if len(request) > 0 {
			peers := s.getBestPeers()
			peers, err = s.getValidCustodyPeers(peers)
			if err != nil {
				return err
			}
			peerCount := len(peers)
			if peerCount == 0 {
				return errors.Wrapf(errNoPeersForPending, "block root=%#x", blkRoot)
			}
			if err := s.sendAndSaveDataColumnSidecars(ctx, request, peers[rand.NewGenerator().Int()%peerCount], b); err != nil {
				return err
			}
		}
	} else {
		request, err := s.pendingBlobsRequestForBlock(blkRoot, b)
		if err != nil {
			return errors.Wrap(err, "pending blobs request for block")
		}

		if len(request) > 0 {
			peers := s.getBestPeers()
			peerCount := len(peers)
			if peerCount == 0 {
				return errors.Wrapf(errNoPeersForPending, "block root=%#x", blkRoot)
			}
			if err := s.sendAndSaveBlobSidecars(ctx, request, peers[rand.NewGenerator().Int()%peerCount], b); err != nil {
				return err
			}
		}
	}

(Only one call to deatures.Get().EnablePeerDAS)

peerCustodiedSubnetCount := params.BeaconConfig().CustodyRequirement
if peerRecord != nil {
// Load the `custody_subnet_count`
// TODO: Do not harcode `custody_subnet_count`
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not hardcoded.

@nisdas nisdas force-pushed the supportDataColumnsInByRootRequests branch from be5f497 to c0e4b3e Compare May 17, 2024 08:32
@nisdas nisdas merged commit 7024767 into peerDAS May 17, 2024
14 of 16 checks passed
@nisdas nisdas deleted the supportDataColumnsInByRootRequests branch May 17, 2024 10:18
nalepae pushed a commit that referenced this pull request May 30, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request May 30, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request Jun 12, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nisdas added a commit that referenced this pull request Jul 4, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request Jul 17, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request Jul 17, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nisdas added a commit that referenced this pull request Jul 18, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request Jul 18, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request Jul 29, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nisdas added a commit that referenced this pull request Aug 14, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nisdas added a commit that referenced this pull request Aug 15, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nisdas added a commit that referenced this pull request Aug 20, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request Aug 27, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request Aug 28, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request Aug 28, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request Oct 7, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request Oct 7, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request Oct 8, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request Oct 23, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request Oct 24, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request Nov 20, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request Nov 22, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request Nov 22, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request Nov 25, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request Nov 25, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
nalepae pushed a commit that referenced this pull request Nov 27, 2024
* Support Data Columns For By Root Requests

* Revert Config Changes

* Fix Panic

* Fix Process Block

* Fix Flags

* Lint

* Support Checkpoint Sync

* Manu's Review

* Add Support For Columns in Remaining Methods

* Unmarshal Uncorrectly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
peerDAS Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants