fix(blob): add fixes along with the fuzzers that discovered them #3732
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
odeke-em
requested review from
renaynay,
Wondertan,
vgonkivs,
distractedm1nd,
walldiss and
cristaloleg
as code owners
This was referenced Sep 16, 2024
odeke-em
force-pushed
the
blob-add-fuzzers
branch
from
af22397
to
9305568
Compare
odeke-em
force-pushed
the
blob-add-fuzzers
branch
from
9305568
to
6f6b900
Compare
cristaloleg
reviewed
Sep 16, 2024
cristaloleg
reviewed
Sep 16, 2024
cristaloleg
reviewed
Sep 16, 2024
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3732 +/- ##
==========================================
+ Coverage 44.83% 46.61% +1.77%
==========================================
Files 265 314 +49
Lines 14620 18014 +3394
==========================================
+ Hits 6555 8397 +1842
- Misses 7313 8598 +1285
- Partials 752 1019 +267 ☔ View full report in Codecov by Sentry. |
walldiss
previously approved these changes
Sep 18, 2024
vgonkivs
previously approved these changes
Sep 18, 2024
|
@odeke-em, do you mind fixing linter? |
|
Hey @odeke-em. Thank you again for your contribution. We want to merge, but we can't until the linter is fixed. We can't push to your branch so we may do another PR cherry-picking your changes if you don't fix linter and rebase. |
auto-merge was automatically disabled
September 25, 2024 02:09
Head branch was pushed to by a user without write access
odeke-em
force-pushed
the
blob-add-fuzzers
branch
from
6f6b900
to
cae6975
Compare
|
Thanks for the ping @Wondertan! I've made an update but I'd like to point out that maybe I am missing context but it is a mistake to prefer relative imports within the same package instead of the whole package: to make things work I had to make repro_test.go skip the absolute import which doesn't look right to make and makes it later cumbersome to copy and paste full working repros diff --git a/blob/repro_test.go b/blob/repro_test.go
index 5489a6a4..42bd966f 100644
--- a/blob/repro_test.go
+++ b/blob/repro_test.go
@@ -4,13 +4,14 @@ import (
"testing"
"github.com/celestiaorg/celestia-app/v2/pkg/proof"
- "github.com/celestiaorg/celestia-node/blob"
"github.com/celestiaorg/nmt"
"github.com/celestiaorg/nmt/pb"
)
// Reported at https://github.com/celestiaorg/celestia-node/issues/3731.
func TestCommitmentProofRowProofVerifyWithEmptyRoot(t *testing.T) {
+ cp := &CommitmentProof{
- cp := &blob.CommitmentProof{
RowProof: proof.RowProof{
Proofs: []*proof.Proof{{}},
},
@@ -23,7 +24,7 @@ func TestCommitmentProofRowProofVerifyWithEmptyRoot(t *testing.T) {
// Reported at https://github.com/celestiaorg/celestia-node/issues/3730.
func TestCommitmentProofRowProofVerify(t *testing.T) {
+ cp := &CommitmentProof{
- cp := &blob.CommitmentProof{
RowProof: proof.RowProof{
Proofs: []*proof.Proof{{}},
},
@@ -36,7 +37,7 @@ func TestCommitmentProofRowProofVerify(t *testing.T) {
// Reported at https://github.com/celestiaorg/celestia-node/issues/3729.
func TestCommitmentProofVerifySliceBound(t *testing.T) {
proof := nmt.ProtoToProof(pb.Proof{End: 1})
+ cp := &CommitmentProof{
- cp := &blob.CommitmentProof{
SubtreeRootProofs: []*nmt.Proof{
&proof,
},
@@ -48,7 +49,7 @@ func TestCommitmentProofVerifySliceBound(t *testing.T) {
// Reported at https://github.com/celestiaorg/celestia-node/issues/3728.
func TestCommitmentProofVerifyZeroSubThreshold(t *testing.T) {
+ cp := new(CommitmentProof)
- cp := new(blob.CommitmentProof)
if _, err := cp.Verify(nil, 0); err == nil {
t.Fatal("expected a non-nil error")
}
@@ -56,8 +57,8 @@ func TestCommitmentProofVerifyZeroSubThreshold(t *testing.T) {
// Reported at https://github.com/celestiaorg/celestia-node/issues/3727.
func TestBlobUnmarshalRepro(t *testing.T) {
+ blob := new(Blob)
+ if err := UnmarshalJSON([]byte("{}")); err == nil {
- blob := new(blob.Blob)
- if err := blob.UnmarshalJSON([]byte("{}")); err == nil {
t.Fatal("expected a non-nil error")
}
} |
odeke-em
force-pushed
the
blob-add-fuzzers
branch
from
cae6975
to
c51b0cb
Compare
@odeke-em, I highly doubt that unless you meant particularly for tests. For tests, yes, we could make every test file its own pkg |
Wondertan
previously approved these changes
Oct 1, 2024
|
Well, linter is still failing |
This changes adds fuzzers+corpra that found some bugs, along with tests and reproducers to catch future regressions. Fixes celestiaorg#3727 Fixes celestiaorg#3728 Fixes celestiaorg#3729 Fixes celestiaorg#3730 Fixes celestiaorg#3731
auto-merge was automatically disabled
October 1, 2024 13:53
Head branch was pushed to by a user without write access
odeke-em
force-pushed
the
blob-add-fuzzers
branch
from
c51b0cb
to
3007ff7
Compare
|
@Wondertan thanks for the review, kindly help me re-run the bots and all tests should pass now! |
Wondertan
approved these changes
Oct 1, 2024
|
@Wondertan perhaps we have a test flake at https://github.com/celestiaorg/celestia-node/actions/runs/11126830327/job/30918109533?pr=3732 2024-10-01T14:05:14.951Z ERROR share/full full/availability.go:71 availability validation failed {"root": "94B65AB76ACAE0512B7A450F77110CD8CE25DD417ECDEDEE26814FAA2FEAF678", "err": "data not found\nread status from stream: protocols not supported: [/private/shrex/v0.1.0/eds_v0]\ndata not found\ncontext deadline exceeded\ncontext deadline exceeded"}
2024-10-01T14:08:14.972Z ERROR pruner/service pruner/find.go:44 failed to get range from header store {"from": 1024, "to": 1025, "error": "header/store: invalid range(1025,1024)"}
2024-10-01T14:08:15.044Z ERROR pruner/service pruner/find.go:44 failed to get range from header store {"from": 1024, "to": 1025, "error": "header/store: invalid range(1025,1024)"}
2024-10-01T14:08:15.102Z ERROR pruner/service pruner/find.go:44 failed to get range from header store {"from": 1024, "to": 1025, "error": "header/store: invalid range(1025,1024)"}
2024-10-01T14:08:15.116Z ERROR pruner/service pruner/find.go:44 failed to get range from header store {"from": 1024, "to": 1025, "error": "header/store: invalid range(1025,1024)"}
2024-10-01T14:08:15.191Z ERROR share/full full/availability.go:71 availability validation failed {"root": "94B65AB76ACAE0512B7A450F77110CD8CE25DD417ECDEDEE26814FAA2FEAF678", "err": "data not found\nopen stream: failed to negotiate protocol: protocols not supported: [/private/shrex/v0.1.0/eds_v0]\ndata not found\ncontext deadline exceeded\ncontext deadline exceeded"}
2024-10-01T14:14:15.236Z ERROR share/full full/availability.go:71 availability validation failed {"root": "94B65AB76ACAE0512B7A450F77110CD8CE25DD417ECDEDEE26814FAA2FEAF678", "err": "data not found\nopen stream: failed to negotiate protocol: protocols not supported: [/private/shrex/v0.1.0/eds_v0]\ndata not found\ncontext deadline exceeded\ncontext deadline exceeded"}
panic: test timed out after 20m0s
running tests:
TestArchivalBlobSync (20m0s)
goroutine 750540 [running]:
testing.(*M).startAlarm.func1()
/opt/hostedtoolcache/go/1.23.1/x64/src/testing/testing.go:2373 +0x385
created by time.goFunc
/opt/hostedtoolcache/go/1.23.1/x64/src/time/sleep.go:215 +0x2d
goroutine 1 [chan receive, 20 minutes]:
testing.(*T).Run(0xc0007ae000, {0x2fdd437?, 0x0?}, 0x384ea60)
/opt/hostedtoolcache/go/1.23.1/x64/src/testing/testing.go:1751 +0x3ab
testing.runTests.func1(0xc0007ae000)
/opt/hostedtoolcache/go/1.23.1/x64/src/testing/testing.go:2168 +0x37
testing.tRunner(0xc0007ae000, 0xc00129fc70)
/opt/hostedtoolcache/go/1.23.1/x64/src/testing/testing.go:1690 +0xf4
testing.runTests(0xc000147158, {0x56acb00, 0x2, 0x2}, {0x475e90?, 0x475afa?, 0x57d7740?})
/opt/hostedtoolcache/go/1.23.1/x64/src/testing/testing.go:2166 +0x43d
testing.(*M).Run(0xc0012da640)
/opt/hostedtoolcache/go/1.23.1/x64/src/testing/testing.go:2034 +0x64a
main.main()
_testmain.go:47 +0x9b
goroutine 5 [select, 20 minutes]:
github.com/desertbit/timer.timerRoutine()
/home/runner/go/pkg/mod/github.com/desertbit/timer@v0.0.0-20180107155436-c41aec40b27f/timers.go:119 +0x9e
created by github.com/desertbit/timer.init.0 in goroutine 1
/home/runner/go/pkg/mod/github.com/desertbit/timer@v0.0.0-20180107155436-c41aec40b27f/timers.go:15 +0x1a
goroutine 7 [select, 20 minutes]:
github.com/ipfs/go-log/writer.(*MirrorWriter).logRoutine(0xc0000c6420)
/home/runner/go/pkg/mod/github.com/ipfs/go-log@v1.0.5/writer/writer.go:71 +0x105
created by github.com/ipfs/go-log/writer.NewMirrorWriter in goroutine 1
/home/runner/go/pkg/mod/github.com/ipfs/go-log@v1.0.5/writer/writer.go:36 +0xbf
goroutine 20 [select]:
github.com/golang/glog.(*fileSink).flushDaemon(0x57d7958)
/home/runner/go/pkg/mod/github.com/golang/glog@v1.2.1/glog_file.go:351 +0xb4
created by github.com/golang/glog.init.1 in goroutine 1
/home/runner/go/pkg/mod/github.com/golang/glog@v1.2.1/glog_file.go:166 +0x126
goroutine 21 [select]:
go.opencensus.io/stats/view.(*worker).start(0xc0003bc200)
/home/runner/go/pkg/mod/go.opencensus.io@v0.24.0/stats/view/worker.go:292 +0x9f
created by go.opencensus.io/stats/view.init.0 in goroutine 1
/home/runner/go/pkg/mod/go.opencensus.io@v0.24.0/stats/view/worker.go:34 +0x8d
goroutine 23 [IO wait, 20 minutes]:
internal/poll.runtime_pollWait(0x7f417b662038, 0x72)
/opt/hostedtoolcache/go/1.23.1/x64/src/runtime/netpoll.go:351 +0x85
internal/poll.(*pollDesc).wait(0xc0006c0880?, 0xc0005bdd50?, 0x0)
/opt/hostedtoolcache/go/1.23.1/x64/src/internal/poll/fd_poll_runtime.go:84 +0x27
internal/poll.(*pollDesc).waitRead(...)
/opt/hostedtoolcache/go/1.23.1/x64/src/internal/poll/fd_poll_runtime.go:89
internal/poll.(*FD).ReadMsg(0xc0006c0880, {0xc0005bdd50, 0x10, 0x10}, {0xc0004f0c28, 0x1000, 0x1000}, 0x40000000)
/opt/hostedtoolcache/go/1.23.1/x64/src/internal/poll/fd_unix.go:302 +0x385
net.(*netFD).readMsg(0xc0006c0880, {0xc0005bdd50?, 0x9?, 0x8d07000000?}, {0xc0004f0c28?, 0x3caae40?, 0x0?}, 0x1?)
/opt/hostedtoolcache/go/1.23.1/x64/src/net/fd_posix.go:78 +0x31
net.(*UnixConn).readMsg(0xc00008a988, {0xc0005bdd50?, 0x41806b?, 0x7f[413](https://github.com/celestiaorg/celestia-node/actions/runs/11126830327/job/30918109533?pr=3732#step:4:414)49c12c8?}, {0xc0004f0c28?, 0xc0006ebdd0?, 0x472c1d?})
/opt/hostedtoolcache/go/1.23.1/x64/src/net/unixsock_posix.go:115 +0x45
net.(*UnixConn).ReadMsgUnix(0xc00008a988, {0xc0005bdd50?, 0x30?, 0x50?}, {0xc0004f0c28?, 0x7f41349c12c8?, 0x18?})
/opt/hostedtoolcache/go/1.23.1/x64/src/net/unixsock.go:143 +0x36
github.com/godbus/dbus.(*oobReader).Read(0xc0004f0c08, {0xc0005bdd50?, 0xc0006ebe30?, 0x413025?})
/home/runner/go/pkg/mod/github.com/godbus/dbus@v0.0.0-20190726142602-4481cbc300e2/transport_unix.go:21 +0x3c
io.ReadAtLeast({0x3c71c00, 0xc0004f0c08}, {0xc0005bdd50, 0x10, 0x10}, 0x10)
/opt/hostedtoolcache/go/1.23.1/x64/src/io/io.go:335 +0x90
io.ReadFull(...)
/opt/hostedtoolcache/go/1.23.1/x64/src/io/io.go:354
github.com/godbus/dbus.(*unixTransport).ReadMessage(0xc000146e10)
/home/runner/go/pkg/mod/github.com/godbus/dbus@v0.0.0-20190726142602-4481cbc300e2/transport_unix.go:91 +0xfe
github.com/godbus/dbus.(*Conn).inWorker(0xc000611050)
/home/runner/go/pkg/mod/github.com/godbus/dbus@v0.0.0-20190726142602-4481cbc300e2/conn.go:294 +0x35
created by github.com/godbus/dbus.(*Conn).Auth in goroutine 1
/home/runner/go/pkg/mod/github.com/godbus/dbus@v0.0.0-20190726142602-4481cbc300e2/auth.go:118 +0x8ee |
cristaloleg
approved these changes
Oct 2, 2024
|
@odeke-em, it is flaky, yes |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes adds fuzzers+corpra that found some bugs,
along with tests and reproducers to catch future regressions.
Fixes #3727
Fixes #3728
Fixes #3729
Fixes #3730
Fixes #3731