Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
6 changes: 5 additions & 1 deletion pkg/p2p/streamtest/streamtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,11 @@ func (r *record) close() {
}

func (r *record) bytes() []byte {
return r.b
r.lock.Lock()
defer r.lock.Unlock()
cp := make([]byte, len(r.b))
copy(cp, r.b)
return cp
}

func (r *record) bytesSize() int {
Expand Down
36 changes: 16 additions & 20 deletions pkg/postage/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,37 +290,33 @@ func (ps *service) HandleStampExpiry(ctx context.Context, id []byte) error {
return nil
}

// removeStampItems
// removeStampItems removes all stamp items belonging to the given batch.
func (ps *service) removeStampItems(ctx context.Context, batchID []byte) error {
ps.logger.Debug("removing expired stamp items", "batchID", hex.EncodeToString(batchID))

deleteItemC := make(chan *StampItem)
go func() {
for item := range deleteItemC {
_ = ps.store.Delete(item)
}
}()

count := 0

defer func() {
close(deleteItemC)
ps.logger.Debug("removed expired stamps", "batchID", hex.EncodeToString(batchID), "count", count)
}()
var toDelete []*StampItem

return ps.store.Iterate(
err := ps.store.Iterate(
storage.Query{
Factory: func() storage.Item { return new(StampItem) },
Prefix: string(batchID),
}, func(result storage.Result) (bool, error) {
select {
case deleteItemC <- result.Entry.(*StampItem):
case <-ctx.Done():
return false, ctx.Err()
if err := ctx.Err(); err != nil {
return false, err
}
count++
toDelete = append(toDelete, result.Entry.(*StampItem))
return false, nil
})
if err != nil {
return err
}

for _, item := range toDelete {
_ = ps.store.Delete(item)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

here the error is swallowed, while other branches return an error. i would at least check whether an error occurred or not, and return an error (i'm not a big fan of errors.Join as a wall of errors is usually not very helpful - having perhaps just the first error returned is fine while continuing the execute the loop)

}

ps.logger.Debug("removed expired stamps", "batchID", hex.EncodeToString(batchID), "count", len(toDelete))
return nil
}

// SetExpired removes all expired batches from the stamp issuers.
Expand Down
2 changes: 1 addition & 1 deletion pkg/postage/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ func TestSetExpired(t *testing.T) {

err = store.Get(itemNotExists)
if err == nil {
t.Fatal(err)
t.Fatal("expected error getting expired stamp item, got nil")
}

testutil.CleanupCloser(t, ps)
Expand Down
Loading