From d8078434046b7b1c1f59f220f5640f2ef2564256 Mon Sep 17 00:00:00 2001 From: Hlib Kanunnikov Date: Mon, 22 Jul 2024 20:44:37 +0200 Subject: [PATCH] refactor(shwap): revert DataRoot -> DataHash (#3585) In https://github.com/celestiaorg/celestia-node/pull/3545, I requested to rename `DataHash` to `DataRoot` to match the name of the original name in the Core's RawHeader. However, I realized it is called `DataHash` in the header, so this PR reverts that change. I think this confusion comes from the fact that we are always referring to this field as `DataRoot` in our discussions, so it feels like this is the correct phrasing, but it's not. It actually does not matter how it's called, honestly, and both are correct in some way, but it's good to be consistent in the code and avoid confusion with tautology. --- share/eds/eds.go | 2 +- share/eds/store.go | 6 +++--- share/new_eds/accessor.go | 4 ++-- share/new_eds/close_once.go | 5 ++--- share/new_eds/close_once_test.go | 2 +- share/new_eds/proofs_cache.go | 4 ++-- share/new_eds/rsmt2d.go | 4 ++-- store/cache/accessor_cache_test.go | 2 +- store/cache/noop.go | 4 ++-- store/file/ods.go | 4 ++-- store/file/q1q4_file.go | 5 ++--- store/store_test.go | 6 +++--- 12 files changed, 23 insertions(+), 25 deletions(-) diff --git a/share/eds/eds.go b/share/eds/eds.go index 7712b3a559..8f129d9ffe 100644 --- a/share/eds/eds.go +++ b/share/eds/eds.go @@ -213,7 +213,7 @@ func rootsToCids(eds *rsmt2d.ExtendedDataSquare) ([]cid.Cid, error) { // ReadEDS reads the first EDS quadrant (1/4) from an io.Reader CAR file. // Only the first quadrant will be read, which represents the original data. -// The returned EDS is guaranteed to be full and valid against the DataRoot, otherwise ReadEDS +// The returned EDS is guaranteed to be full and valid against the DataHash, otherwise ReadEDS // errors. func ReadEDS(ctx context.Context, r io.Reader, root share.DataHash) (eds *rsmt2d.ExtendedDataSquare, err error) { _, span := tracer.Start(ctx, "read-eds") diff --git a/share/eds/store.go b/share/eds/store.go index 493bb0fd9e..f97ac4c12b 100644 --- a/share/eds/store.go +++ b/share/eds/store.go @@ -208,7 +208,7 @@ func (s *Store) watchForFailures(ctx context.Context) { } } -// Put stores the given data square with DataRoot's hash as a key. +// Put stores the given data square with DataHash's hash as a key. // // The square is verified on the Exchange level, and Put only stores the square, trusting it. // The resulting file stores all the shares and NMT Merkle Proofs of the EDS. @@ -335,7 +335,7 @@ func trackLateResult(opName string, res <-chan dagstore.ShardResult, metrics *me } } -// GetCAR takes a DataRoot and returns a buffered reader to the respective EDS serialized as a +// GetCAR takes a DataHash and returns a buffered reader to the respective EDS serialized as a // CARv1 file. // The Reader strictly reads the CAR header and first quadrant (1/4) of the EDS, omitting all the // NMT Merkle proofs. Integrity of the store data is not verified. @@ -522,7 +522,7 @@ func (s *Store) remove(ctx context.Context, root share.DataHash) (err error) { return nil } -// Get reads EDS out of Store by given DataRoot. +// Get reads EDS out of Store by given DataHash. // // It reads only one quadrant(1/4) of the EDS and verifies the integrity of the stored data by // recomputing it. diff --git a/share/new_eds/accessor.go b/share/new_eds/accessor.go index 30b671e302..66296ad95b 100644 --- a/share/new_eds/accessor.go +++ b/share/new_eds/accessor.go @@ -14,8 +14,8 @@ import ( type Accessor interface { // Size returns square size of the Accessor. Size(ctx context.Context) int - // DataRoot returns data hash of the Accessor. - DataRoot(ctx context.Context) (share.DataHash, error) + // DataHash returns data hash of the Accessor. + DataHash(ctx context.Context) (share.DataHash, error) // Sample returns share and corresponding proof for row and column indices. Implementation can // choose which axis to use for proof. Chosen axis for proof should be indicated in the returned // Sample. diff --git a/share/new_eds/close_once.go b/share/new_eds/close_once.go index 3a6906bfac..6297a32e52 100644 --- a/share/new_eds/close_once.go +++ b/share/new_eds/close_once.go @@ -42,12 +42,11 @@ func (c *closeOnce) Size(ctx context.Context) int { return c.f.Size(ctx) } -// DataRoot returns root hash of Accessor's underlying EDS. -func (c *closeOnce) DataRoot(ctx context.Context) (share.DataHash, error) { +func (c *closeOnce) DataHash(ctx context.Context) (share.DataHash, error) { if c.closed.Load() { return nil, errAccessorClosed } - return c.f.DataRoot(ctx) + return c.f.DataHash(ctx) } func (c *closeOnce) Sample(ctx context.Context, rowIdx, colIdx int) (shwap.Sample, error) { diff --git a/share/new_eds/close_once_test.go b/share/new_eds/close_once_test.go index df5ef1731c..26b9a9b24a 100644 --- a/share/new_eds/close_once_test.go +++ b/share/new_eds/close_once_test.go @@ -50,7 +50,7 @@ func (s *stubEdsAccessorCloser) Size(context.Context) int { return 0 } -func (s *stubEdsAccessorCloser) DataRoot(context.Context) (share.DataHash, error) { +func (s *stubEdsAccessorCloser) DataHash(context.Context) (share.DataHash, error) { return nil, nil } diff --git a/share/new_eds/proofs_cache.go b/share/new_eds/proofs_cache.go index 7494b31025..24f068caed 100644 --- a/share/new_eds/proofs_cache.go +++ b/share/new_eds/proofs_cache.go @@ -77,8 +77,8 @@ func (c *proofsCache) Size(ctx context.Context) int { return int(size) } -func (c *proofsCache) DataRoot(ctx context.Context) (share.DataHash, error) { - return c.inner.DataRoot(ctx) +func (c *proofsCache) DataHash(ctx context.Context) (share.DataHash, error) { + return c.inner.DataHash(ctx) } func (c *proofsCache) Sample(ctx context.Context, rowIdx, colIdx int) (shwap.Sample, error) { diff --git a/share/new_eds/rsmt2d.go b/share/new_eds/rsmt2d.go index 3097be1259..b18700f6c4 100644 --- a/share/new_eds/rsmt2d.go +++ b/share/new_eds/rsmt2d.go @@ -24,8 +24,8 @@ func (eds *Rsmt2D) Size(context.Context) int { return int(eds.Width()) } -// DataRoot returns data hash of the Accessor. -func (eds *Rsmt2D) DataRoot(context.Context) (share.DataHash, error) { +// DataHash returns data hash of the Accessor. +func (eds *Rsmt2D) DataHash(context.Context) (share.DataHash, error) { roots, err := share.NewAxisRoots(eds.ExtendedDataSquare) if err != nil { return share.DataHash{}, fmt.Errorf("while creating data root: %w", err) diff --git a/store/cache/accessor_cache_test.go b/store/cache/accessor_cache_test.go index 7b8261c72c..b37d852a80 100644 --- a/store/cache/accessor_cache_test.go +++ b/store/cache/accessor_cache_test.go @@ -300,7 +300,7 @@ func (m *mockAccessor) Size(context.Context) int { panic("implement me") } -func (m *mockAccessor) DataRoot(context.Context) (share.DataHash, error) { +func (m *mockAccessor) DataHash(context.Context) (share.DataHash, error) { panic("implement me") } diff --git a/store/cache/noop.go b/store/cache/noop.go index 045369a635..f327ec5c69 100644 --- a/store/cache/noop.go +++ b/store/cache/noop.go @@ -46,8 +46,8 @@ func (n NoopFile) Size(context.Context) int { return 0 } -// DataRoot returns root hash of Accessor's underlying EDS. -func (n NoopFile) DataRoot(context.Context) (share.DataHash, error) { +// DataHash returns root hash of Accessor's underlying EDS. +func (n NoopFile) DataHash(context.Context) (share.DataHash, error) { return nil, nil } diff --git a/store/file/ods.go b/store/file/ods.go index 2128942190..ff39a77684 100644 --- a/store/file/ods.go +++ b/store/file/ods.go @@ -120,8 +120,8 @@ func (f *ODSFile) size() int { return int(f.hdr.squareSize) } -// DataRoot returns root hash of Accessor's underlying EDS. -func (f *ODSFile) DataRoot(context.Context) (share.DataHash, error) { +// DataHash returns root hash of Accessor's underlying EDS. +func (f *ODSFile) DataHash(context.Context) (share.DataHash, error) { return f.hdr.datahash, nil } diff --git a/store/file/q1q4_file.go b/store/file/q1q4_file.go index 1eb242f749..21e72e2072 100644 --- a/store/file/q1q4_file.go +++ b/store/file/q1q4_file.go @@ -53,9 +53,8 @@ func (f *Q1Q4File) Size(ctx context.Context) int { return f.ods.Size(ctx) } -// DataRoot returns root hash of Accessor's underlying EDS. -func (f *Q1Q4File) DataRoot(ctx context.Context) (share.DataHash, error) { - return f.ods.DataRoot(ctx) +func (f *Q1Q4File) DataHash(ctx context.Context) (share.DataHash, error) { + return f.ods.DataHash(ctx) } func (f *Q1Q4File) Sample(ctx context.Context, rowIdx, colIdx int) (shwap.Sample, error) { diff --git a/store/store_test.go b/store/store_test.go index d0a2ec644a..c60fb8c4a2 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -184,7 +184,7 @@ func TestEDSStore(t *testing.T) { // assert that the empty file is, in fact, empty f, err := edsStore.GetByDataRoot(ctx, roots.Hash()) require.NoError(t, err) - hash, err := f.DataRoot(ctx) + hash, err := f.DataHash(ctx) require.NoError(t, err) require.True(t, hash.IsEmptyEDS()) }) @@ -206,7 +206,7 @@ func TestEDSStore(t *testing.T) { // assert that the empty file can be accessed by height f, err := edsStore.GetByHeight(ctx, height) require.NoError(t, err) - hash, err := f.DataRoot(ctx) + hash, err := f.DataHash(ctx) require.NoError(t, err) require.True(t, hash.IsEmptyEDS()) require.NoError(t, f.Close()) @@ -237,7 +237,7 @@ func TestEDSStore(t *testing.T) { for i := from; i <= to; i++ { f, err := edsStore.GetByHeight(ctx, uint64(i)) require.NoError(t, err) - hash, err := f.DataRoot(ctx) + hash, err := f.DataHash(ctx) require.NoError(t, err) require.True(t, hash.IsEmptyEDS()) require.NoError(t, f.Close())