chore: ignore double sign error #436
Conversation
9e02d0a to
49be4b7
Compare
504d179 to
72d0beb
Compare
57584d6 to
f1d1741
Compare
f1d1741 to
eadcca4
Compare
|
Will wait for a second approval as it's a critical one @KonradStaniec |
KonradStaniec
left a comment
There was a problem hiding this comment.
code looks good 👍 just some clarifying questions
I think next step, will be testing this change on devent ?
| if len(validBlocks) == 0 { | ||
| fp.logger.Info("all blocks were skipped due to double sign errors") | ||
|
|
||
| return nil, nil |
There was a problem hiding this comment.
double checking, is the calling code ready to handle nil, nil response ?
second question, if all blocks are marked as double signable shouldn't we update fp state to latest block:
highBlock := blocks[len(blocks)-1]
fp.MustUpdateStateAfterFinalitySigSubmission(highBlock.Height)?
There was a problem hiding this comment.
double checking, is the calling code ready to handle nil, nil response ?
Yes, it is bubbled up to
I added contract in the doc string
if all blocks are marked as double signable shouldn't we update fp state to latest block
Yes, it should be set as the latest block. Isn't it what the PR does? The batch that is actually sent is validBlocks
| // TestSkippingDoubleSignError tests the scenario where the finality-provider | ||
| // should skip the block when encountering a double sign request from the | ||
| // eots manager | ||
| func TestSkippingDoubleSignError(t *testing.T) { |
There was a problem hiding this comment.
when I run e2e test I see some stacktrace being thrown:
2025-04-09T09:02:10.933+0200 ERROR eotsmanager/localmanager.go:212 double sign requested {"eots_pk": "f5d32d55464d5cb401d5283efd788b607cb3c1614512d93243b42da1af30a5e6", "hash": "0000000000000011b8073846dfe718a0f13840dc27688b9348ed07d0c172fd0a9eb461679ea702c3", "height": 17, "chainID": "chain-test"}
github.com/babylonlabs-io/finality-provider/eotsmanager.(*LocalEOTSManager).SignEOTS
/Users/konradstaniec/Work/babylon/finality-provider/eotsmanager/localmanager.go:212
github.com/babylonlabs-io/finality-provider/eotsmanager/service.(*rpcServer).SignEOTS
/Users/konradstaniec/Work/babylon/finality-provider/eotsmanager/service/rpcserver.go:69
github.com/babylonlabs-io/finality-provider/eotsmanager/proto._EOTSManager_SignEOTS_Handler
/Users/konradstaniec/Work/babylon/finality-provider/eotsmanager/proto/eotsmanager_grpc.pb.go:221
google.golang.org/grpc.(*Server).processUnaryRPC
/Users/konradstaniec/go/pkg/mod/google.golang.org/grpc@v1.70.0/server.go:1400
google.golang.org/grpc.(*Server).handleStream
/Users/konradstaniec/go/pkg/mod/google.golang.org/grpc@v1.70.0/server.go:1810
google.golang.org/grpc.(*Server).serveStreams.func2.1
/Users/konradstaniec/go/pkg/mod/google.golang.org/grpc@v1.70.0/server.go:1030
babylon_test_manager.go:371: the fp voted at height 18
is this expected ?
There was a problem hiding this comment.
Yes. We log it as ERROR level.
finality-provider/eotsmanager/localmanager.go
Line 212 in 92779f9
Probably. I was thinking how to test this scenario. In the e2e test of this PR, we stop the fp for a while and make a manual submission, then restart. We probably can do the same on devnet? |
I would say so. This seems to be easiest way to test this scenario |
Closes https://github.com/babylonlabs-io/pm/issues/319