universe: update pushProofToFederation to make sure all servers are tried #528
Conversation
| log.Warnf("cannot push proof unable to connect "+ | ||
| "to remote server(%v): %v", addr.HostStr(), | ||
| err) | ||
| return nil | ||
| } | ||
|
|
||
| _, err = remoteUniverseServer.RegisterIssuance( |
There was a problem hiding this comment.
I think that if we encounter and return an error at this point then the errgroup.Group of the fn.ParSlice function call below will cancel the context for all servers.
We should probably warn rather than return an error here also.
There was a problem hiding this comment.
I think if you can't register a proof with an universe after being able to connect to that universe, I think that might warrant bubbling up the error. As that could point to something being actually wrong (as opposed to the server just being offline).
There was a problem hiding this comment.
An attacker might run a universe server that would accept a connection but maliciously returns a failure when attempting to submit proofs to it (perhaps for only certain coins etc). Which would interrupt a user's ability to register a proof with all other servers.
I think we should bubble up an error if we fail in any way for every universe server. I don't think fn.ParSlice (because of errgroup.Group) is the right way to go in the long term because it's fragile. But i think further changes belong in a different PR.
…ried In this commit, we fix a bug that would cause universe proof push to fail for all servers, if anyone of them was unreachable, or rejected the proof. If we return `err` here, then eventually the context will be read by other goroutines, which is already cancelled at this point. This'll cause the push attempt to stop short. We now long and return `nil` so the other goroutines will continue. See lightninglabs#527 for more details.
Roasbeef
force-pushed
the
never-gonna-give-up-universe-push
branch
from
da192f6 to
6444939
Compare
In this commit, we fix a bug that would cause universe proof push to fail for all servers, if anyone of them was unreachable, or rejected the proof.
If we return
errhere, then eventually the context will be read by other goroutines, which is already cancelled at this point. This'll cause the push attempt to stop short.We now long and return
nilso the other goroutines will continue.See #527 for more details.