Skip to content

merkledb -- remove unneeded return values #1959

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

Merged
merged 2 commits into from
Sep 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions x/merkledb/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -1085,9 +1085,7 @@ func (db *merkleDB) initializeRootIfNeeded() (ids.ID, error) {
}
if err == nil {
// Root already exists, so calculate its id
if err := db.root.calculateID(db.metrics); err != nil {
return ids.Empty, err
}
db.root.calculateID(db.metrics)
return db.root.id, nil
}
if err != database.ErrNotFound {
Expand All @@ -1098,9 +1096,7 @@ func (db *merkleDB) initializeRootIfNeeded() (ids.ID, error) {
db.root = newNode(nil, RootPath)

// update its ID
if err := db.root.calculateID(db.metrics); err != nil {
return ids.Empty, err
}
db.root.calculateID(db.metrics)

if err := db.intermediateNodeDB.Put(RootPath, db.root); err != nil {
return ids.Empty, err
Expand Down
13 changes: 5 additions & 8 deletions x/merkledb/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,21 +98,18 @@ func (n *node) onNodeChanged() {
}

// Returns and caches the ID of this node.
func (n *node) calculateID(metrics merkleMetrics) error {
func (n *node) calculateID(metrics merkleMetrics) {
if n.id != ids.Empty {
return nil
return
}

hv := &hashValues{
metrics.HashCalculated()
bytes := codec.encodeHashValues(&hashValues{
Children: n.children,
Value: n.valueDigest,
Key: n.key.Serialize(),
}

bytes := codec.encodeHashValues(hv)
metrics.HashCalculated()
})
n.id = hashing.ComputeHash256Array(bytes)
return nil
}

// Set [n]'s value to [val].
Expand Down
6 changes: 3 additions & 3 deletions x/merkledb/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func Test_Node_Marshal(t *testing.T) {
childNode.setValue(maybe.Some([]byte("value")))
require.NotNil(t, childNode)

require.NoError(t, childNode.calculateID(&mockMetrics{}))
childNode.calculateID(&mockMetrics{})
root.addChild(childNode)

data := root.bytes()
Expand All @@ -45,15 +45,15 @@ func Test_Node_Marshal_Errors(t *testing.T) {
childNode1.setValue(maybe.Some([]byte("value1")))
require.NotNil(t, childNode1)

require.NoError(t, childNode1.calculateID(&mockMetrics{}))
childNode1.calculateID(&mockMetrics{})
root.addChild(childNode1)

fullpath = newPath([]byte{237})
childNode2 := newNode(root, fullpath)
childNode2.setValue(maybe.Some([]byte("value2")))
require.NotNil(t, childNode2)

require.NoError(t, childNode2.calculateID(&mockMetrics{}))
childNode2.calculateID(&mockMetrics{})
root.addChild(childNode2)

data := root.bytes()
Expand Down
24 changes: 10 additions & 14 deletions x/merkledb/trieview.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,9 +242,7 @@ func (t *trieView) calculateNodeIDs(ctx context.Context) error {
// [eg] limits the number of goroutines we start.
var eg errgroup.Group
eg.SetLimit(t.db.rootGenConcurrency)
if err = t.calculateNodeIDsHelper(ctx, t.root, &eg); err != nil {
return
}
t.calculateNodeIDsHelper(ctx, t.root, &eg)
if err = eg.Wait(); err != nil {
return
}
Expand All @@ -261,7 +259,7 @@ func (t *trieView) calculateNodeIDs(ctx context.Context) error {

// Calculates the ID of all descendants of [n] which need to be recalculated,
// and then calculates the ID of [n] itself.
func (t *trieView) calculateNodeIDsHelper(ctx context.Context, n *node, eg *errgroup.Group) error {
func (t *trieView) calculateNodeIDsHelper(ctx context.Context, n *node, eg *errgroup.Group) {
var (
// We use [wg] to wait until all descendants of [n] have been updated.
// Note we can't wait on [eg] because [eg] may have started goroutines
Expand All @@ -281,24 +279,22 @@ func (t *trieView) calculateNodeIDsHelper(ctx context.Context, n *node, eg *errg
}

wg.Add(1)
updateChild := func() error {
updateChild := func() {
defer wg.Done()

if err := t.calculateNodeIDsHelper(ctx, childNodeChange.after, eg); err != nil {
return err
}
t.calculateNodeIDsHelper(ctx, childNodeChange.after, eg)

// Note that this will never block
updatedChildren <- childNodeChange.after
return nil
}

// Try updating the child and its descendants in a goroutine.
if ok := eg.TryGo(updateChild); !ok {
if ok := eg.TryGo(func() error {
updateChild()
return nil
}); !ok {
// We're at the goroutine limit; do the work in this goroutine.
if err := updateChild(); err != nil {
return err
}
updateChild()
}
}

Expand All @@ -311,7 +307,7 @@ func (t *trieView) calculateNodeIDsHelper(ctx context.Context, n *node, eg *errg
}

// The IDs [n]'s descendants are up to date so we can calculate [n]'s ID.
return n.calculateID(t.db.metrics)
n.calculateID(t.db.metrics)
}

// GetProof returns a proof that [bytesPath] is in or not in trie [t].
Expand Down