PeerDAS: Implement reconstruction. #14036

nalepae · 2024-05-22T11:23:04Z

Please read commit by commit.

This pull requests:

Adds some logs and error wrappings.
Fix a bug in initial sync regarding data columns.
Implement data columns reconstruction.

About data columns reconstruction:
If half of the total number of columns is received via gossip, the reconstruction is triggered. This means that data columns reconstruction is automatic for super nodes.
All columns are saved in the database.
Right now, reconstructed columns are NOT sent back to peers.

New added debug logs:

["2024-05-24 12:24:34"]  DEBUG sync: "Recovered blob" elapsed=30.6185ms index=1 root=1627eb76c870c0fc3011a41192b6d67bd1de1665d1cc8d522556be8bdb50b245
["2024-05-24 12:24:34"]  DEBUG sync: "Recovered blob" elapsed=31.680583ms index=2 root=1627eb76c870c0fc3011a41192b6d67bd1de1665d1cc8d522556be8bdb50b245
["2024-05-24 12:24:34"]  DEBUG sync: "Recovered blob" elapsed=32.598375ms index=3 root=1627eb76c870c0fc3011a41192b6d67bd1de1665d1cc8d522556be8bdb50b245
["2024-05-24 12:24:34"]  DEBUG sync: "Recovered blob" elapsed=30.318625ms index=4 root=1627eb76c870c0fc3011a41192b6d67bd1de1665d1cc8d522556be8bdb50b245
["2024-05-24 12:24:34"]  DEBUG sync: "Recovered blob" elapsed=30.1115ms index=5 root=1627eb76c870c0fc3011a41192b6d67bd1de1665d1cc8d522556be8bdb50b245
["2024-05-24 12:24:35"]  DEBUG sync: "Data columns reconstructed successfully" root=1627eb76c870c0fc3011a41192b6d67bd1de1665d1cc8d522556be8bdb50b245

beacon-chain/sync/rpc_data_column_sidecars_by_root.go

…mberOfColumns]bool`.

…ReadOnlySignedBeaconBlock`. We don't need any of the non read-only methods.

If we attempt to save an already existing data column sidecar, a debug log was printed. This case could be quite common now with the data column reconstruction enabled.

saolyn

LGTM

nisdas · 2024-05-28T08:22:56Z

beacon-chain/sync/data_columns_reconstruct.go

+
+// recoverBlobs recovers the blobs from the data column sidecars.
+func recoverBlobs(
+	dataColumnSideCars []*ethpb.DataColumnSidecar,


if the column count is < 50% we should return an error early as reconstruction is not possible

Reconstruction is not possible with the current chosen (2) replication factor.
Maybe one day this replication factor will change to an other value.

In the replication factor changes, we well need to:

update the corresponding KZG library we use,

update reconstructDataColumns (where there is already a check), and

update the early return condition.

Trying to reconstruct with less than the minimum amount of data needed will make RecoverAllCells to return on error. (And thus, the recoverBlobs function return on error as well.)

Because recoverBlobs is a private function, only called by reconstructDataColumns (which include the replication factor check), my opinion is it is not necessary to add a new return early check.

nisdas · 2024-05-28T08:28:31Z

beacon-chain/sync/data_columns_reconstruct.go

+		cellsId := make([]uint64, 0, columnsCount)
+		cKzgCells := make([]cKzg4844.Cell, 0, columnsCount)
+
+		for _, sidecar := range dataColumnSideCars {


are the sidecars assumed to be inorder ? otherwise you would have something looking very different

There is no such an assumption.

However, the order in cellIsd must be the same than the order in cKzgCells, since they are given to

recoveredCells, err := cKzg4844.RecoverAllCells(cellsId, cKzgCells)

The order itself does not matter, but the order between cellsId and cKzgCells does.
(And this order is respected in the current implementation.)

nisdas · 2024-05-28T08:42:04Z

beacon-chain/sync/data_columns_reconstruct.go

+	}
+
+	// Recover blobs.
+	recoveredBlobs, err := recoverBlobs(dataColumnSideCars, storedColumnsCount, blockRoot)


Its very expensive to go from columns -> blobs and then back to columns. Once we recover all the cells, we can simply use that to reseed the missing columns.

Actually we don't really do columns -> blobs -> columns but (partial) dataColumnsSidecars -> blobs -> (complete) dataColumnsSidecars.

To get the complete dataColumnsSidecars, we need the KZG proofs, which are computed by blobs, using cKzg4844.ComputeCellsAndKZGProofs(blob).

Do you see a way to compute these proofs without going trough blobs?

The KZG Proofs are computed for all blobs no ? for each column sidecar. You can simply reuse the previous commitments here from the earlier columns.

* Wrap errors, add logs. * `missingColumnRequest`: Fix blobs <-> data columns mix. * `ColumnIndices`: Return `map[uint64]bool` instead of `[fieldparams.NumberOfColumns]bool`. * `DataColumnSidecars`: `interfaces.SignedBeaconBlock` ==> `interfaces.ReadOnlySignedBeaconBlock`. We don't need any of the non read-only methods. * Fix comments. * `handleUnblidedBlock` ==> `handleUnblindedBlock`. * `SaveDataColumn`: Move log from debug to trace. If we attempt to save an already existing data column sidecar, a debug log was printed. This case could be quite common now with the data column reconstruction enabled. * `sampling_data_columns.go` --> `data_columns_sampling.go`. * Reconstruct data columns.

nalepae requested a review from a team as a code owner May 22, 2024 11:23

nalepae requested review from kasey, rauljordan and saolyn and removed request for a team May 22, 2024 11:23

nalepae force-pushed the peerDASLogs branch 2 times, most recently from f804c5a to 0df72de Compare May 22, 2024 11:32

saolyn reviewed May 22, 2024

View reviewed changes

beacon-chain/sync/rpc_data_column_sidecars_by_root.go Outdated Show resolved Hide resolved

nalepae force-pushed the peerDASLogs branch from cdbae5e to 8f60e35 Compare May 24, 2024 07:49

nalepae added 2 commits May 24, 2024 14:18

Wrap errors, add logs.

77b66c3

missingColumnRequest: Fix blobs <-> data columns mix.

9e8495d

nalepae force-pushed the peerDASLogs branch from 8f60e35 to 8f1bb15 Compare May 24, 2024 12:23

nalepae changed the title ~~Wrap errors, add logs.~~ PeerDAS: Implement reconstruction. May 24, 2024

nalepae added 6 commits May 24, 2024 14:31

ColumnIndices: Return map[uint64]bool instead of `[fieldparams.Nu…

4fcd018

…mberOfColumns]bool`.

DataColumnSidecars: interfaces.SignedBeaconBlock ==> `interfaces.…

c118683

…ReadOnlySignedBeaconBlock`. We don't need any of the non read-only methods.

Fix comments.

08c86c6

handleUnblidedBlock ==> handleUnblindedBlock.

d3ed92b

SaveDataColumn: Move log from debug to trace.

3a0f9a6

If we attempt to save an already existing data column sidecar, a debug log was printed. This case could be quite common now with the data column reconstruction enabled.

sampling_data_columns.go --> data_columns_sampling.go.

0fabbef

nalepae force-pushed the peerDASLogs branch from 8f1bb15 to c43358c Compare May 24, 2024 12:31

Reconstruct data columns.

bce1879

nalepae force-pushed the peerDASLogs branch from c43358c to bce1879 Compare May 24, 2024 12:34

saolyn approved these changes May 25, 2024

View reviewed changes

nisdas reviewed May 28, 2024

View reviewed changes

nisdas approved these changes May 29, 2024

View reviewed changes

nalepae merged commit 25bd4e7 into peerDAS May 29, 2024
13 of 16 checks passed

nalepae deleted the peerDASLogs branch May 29, 2024 08:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PeerDAS: Implement reconstruction. #14036

PeerDAS: Implement reconstruction. #14036

nalepae commented May 22, 2024 •

edited

Loading

saolyn left a comment

nisdas May 28, 2024

nalepae May 28, 2024

nisdas May 28, 2024

nalepae May 28, 2024 •

edited

Loading

nisdas May 28, 2024

nalepae May 28, 2024

nisdas May 28, 2024

PeerDAS: Implement reconstruction. #14036

PeerDAS: Implement reconstruction. #14036

Conversation

nalepae commented May 22, 2024 • edited Loading

saolyn left a comment

Choose a reason for hiding this comment

nisdas May 28, 2024

Choose a reason for hiding this comment

nalepae May 28, 2024

Choose a reason for hiding this comment

nisdas May 28, 2024

Choose a reason for hiding this comment

nalepae May 28, 2024 • edited Loading

Choose a reason for hiding this comment

nisdas May 28, 2024

Choose a reason for hiding this comment

nalepae May 28, 2024

Choose a reason for hiding this comment

nisdas May 28, 2024

Choose a reason for hiding this comment

nalepae commented May 22, 2024 •

edited

Loading

nalepae May 28, 2024 •

edited

Loading