[Merged by Bors] - Do not try to publish proofs for malicious ATXs during sync #5586
Conversation
Publishing is blocked during sync because Syncer::ListenToATXGossip() returns false, and thus every malicious ATX being synced was causing an error resulting in an interruption of sync.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #5586 +/- ##
=======================================
Coverage 79.6% 79.7%
=======================================
Files 270 270
Lines 27186 27199 +13
=======================================
+ Hits 21667 21688 +21
+ Misses 3973 3967 -6
+ Partials 1546 1544 -2 ☔ View full report in Codecov by Sentry. |
|
bors try |
tryBuild succeeded: |
activation/handler.go
Outdated
| } | ||
| encodedProof, err := codec.Encode(&gossip) | ||
| if err != nil { | ||
| h.log.With().Fatal("failed to encode malfeasance gossip", log.Err(err)) |
There was a problem hiding this comment.
maybe MustEncode, since the code was changed
| } | ||
| if err = h.publisher.Publish(ctx, pubsub.MalfeasanceProof, encodedProof); err != nil { | ||
| h.log.With().Error("failed to broadcast malfeasance proof", log.Err(err)) | ||
| return fmt.Errorf("broadcast atx malfeasance proof: %w", err) |
There was a problem hiding this comment.
is the log 1 line above necessary if we return same error up the stack?
There was a problem hiding this comment.
The error message from the handlers was being ignored in pubsub code:
go-spacemesh/p2p/pubsub/wrapper.go
Lines 41 to 51 in 6cd369d
I've added debug logging there, but IIUC failing to be able to broadcast the malfeasance proof should be more than a silent validation error by default, so I kept logging there, too.
| h.inProgressMu.Lock() | ||
| defer h.inProgressMu.Unlock() | ||
| for _, ch := range h.inProgress[atx.ID()] { | ||
| ch <- err | ||
| close(ch) | ||
| } | ||
| delete(h.inProgress, atx.ID()) | ||
| return err | ||
| return proof, err |
There was a problem hiding this comment.
h.processATX() call above that may return an error, and that error needs to be sent over an "inProgress" channel, but it also needs to be returned so that HandleSyncedAtx() and HandleGossipAtx() don't swallow errors
|
@ivan4th should we merge the change? |
|
Yes, let's merge |
|
bors merge |
## Motivation Publishing is blocked during sync because `Syncer::ListenToATXGossip()` returns false, and thus every malicious ATX being synced is causing an error resulting in an interruption of sync.
|
Pull request successfully merged into develop. Build succeeded: |
spacemesh-bors
bot
changed the title
Publishing is blocked during sync because `Syncer::ListenToATXGossip()` returns false, and thus every malicious ATX being synced is causing an error resulting in an interruption of sync.
Publishing is blocked during sync because `Syncer::ListenToATXGossip()` returns false, and thus every malicious ATX being synced is causing an error resulting in an interruption of sync.
Publishing is blocked during sync because `Syncer::ListenToATXGossip()` returns false, and thus every malicious ATX being synced is causing an error resulting in an interruption of sync.
Backport #5586: Do not try to publish proofs for malicious ATXs during sync
Motivation
Publishing is blocked during sync because
Syncer::ListenToATXGossip()returns false, and thus every malicious ATX being synced is causing an error resulting in an interruption of sync.Description
Do not try to publish ATX malfeasance proofs during sync.
Test Plan
Sync a test node, look for sync interruptions / errors