From f48dffe0e683c529c0203e106cd2b654a7df9add Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Mon, 15 Nov 2021 17:32:14 +0300 Subject: [PATCH 1/2] core: fix fallback VUB value in notary test Serializing/deserializing the payload yields this: Error: Received unexpected error: both main and fallback transactions should have the same ValidUntil value --- pkg/core/notary_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/core/notary_test.go b/pkg/core/notary_test.go index 784b477db..7b35be0fd 100644 --- a/pkg/core/notary_test.go +++ b/pkg/core/notary_test.go @@ -146,7 +146,7 @@ func TestNotary(t *testing.T) { fallback.Nonce = nonce nonce++ fallback.SystemFee = 1_0000_0000 - fallback.ValidUntilBlock = bc.BlockHeight() + 50 + fallback.ValidUntilBlock = bc.BlockHeight() + 2*nvbDiffFallback fallback.Signers = []transaction.Signer{ { Account: bc.GetNotaryContractScriptHash(), From 65016e807031f00cc9126863e829d5015c46dc65 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Mon, 15 Nov 2021 17:33:02 +0300 Subject: [PATCH 2/2] core: fix data race in notary test I don't think it's possible with regular service functioning, but it happens during testing because of pointer reuse: WARNING: DATA RACE Read at 0x00c003a0e3f0 by goroutine 114: github.com/nspcc-dev/neo-go/pkg/services/notary.(*Notary).verifyIncompleteWitnesses() /home/runner/work/neo-go/neo-go/pkg/services/notary/notary.go:441 +0x1dc github.com/nspcc-dev/neo-go/pkg/services/notary.(*Notary).OnNewRequest() /home/runner/work/neo-go/neo-go/pkg/services/notary/notary.go:188 +0x205 github.com/nspcc-dev/neo-go/pkg/core.TestNotary.func11() /home/runner/work/neo-go/neo-go/pkg/core/notary_test.go:347 +0x612 github.com/nspcc-dev/neo-go/pkg/core.TestNotary() /home/runner/work/neo-go/neo-go/pkg/core/notary_test.go:443 +0xe33 testing.tRunner() /opt/hostedtoolcache/go/1.16.10/x64/src/testing/testing.go:1193 +0x202 Previous write at 0x00c003a0e3f0 by goroutine 104: github.com/nspcc-dev/neo-go/pkg/services/notary.(*Notary).finalize() /home/runner/work/neo-go/neo-go/pkg/services/notary/notary.go:338 +0x50a github.com/nspcc-dev/neo-go/pkg/services/notary.(*Notary).PostPersist() /home/runner/work/neo-go/neo-go/pkg/services/notary/notary.go:314 +0x297 github.com/nspcc-dev/neo-go/pkg/services/notary.(*Notary).Run() /home/runner/work/neo-go/neo-go/pkg/services/notary/notary.go:169 +0x4a7 --- pkg/core/notary_test.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/pkg/core/notary_test.go b/pkg/core/notary_test.go index 7b35be0fd..2ca1d8e89 100644 --- a/pkg/core/notary_test.go +++ b/pkg/core/notary_test.go @@ -55,6 +55,18 @@ func getTestNotary(t *testing.T, bc *Blockchain, walletPath, pass string, onTx f return w.Accounts[0], ntr, mp } +// dupNotaryRequest duplicates notary request by serializing/deserializing it. Use +// it to avoid data races when reusing the same payload. Normal OnNewRequest handler +// never receives the same (as in the same pointer) payload multiple times, even if +// the contents is the same it would be a separate buffer. +func dupNotaryRequest(t *testing.T, p *payload.P2PNotaryRequest) *payload.P2PNotaryRequest { + b, err := p.Bytes() + require.NoError(t, err) + r, err := payload.NewP2PNotaryRequestFromBytes(b) + require.NoError(t, err) + return r +} + func TestNotary(t *testing.T) { bc := newTestChain(t) var ( @@ -344,7 +356,7 @@ func TestNotary(t *testing.T) { completedCount := len(completedTxes) // check that the same request won't be processed twice - ntr1.OnNewRequest(requests[sendOrder[i]]) + ntr1.OnNewRequest(dupNotaryRequest(t, requests[sendOrder[i]])) checkMainTx(t, requesters, requests, i+1, shouldComplete) require.Equal(t, completedCount, len(completedTxes)) } @@ -380,7 +392,7 @@ func TestNotary(t *testing.T) { checkMainTx(t, requesters, submittedRequests, i+1, shouldComplete) // check that the same request won't be processed twice - ntr1.OnNewRequest(requests[sendOrder[i]]) + ntr1.OnNewRequest(dupNotaryRequest(t, requests[sendOrder[i]])) checkMainTx(t, requesters, submittedRequests, i+1, shouldComplete) } @@ -424,7 +436,7 @@ func TestNotary(t *testing.T) { completedCount := len(completedTxes) // check that the same request won't be processed twice - ntr1.OnNewRequest(requests[i]) + ntr1.OnNewRequest(dupNotaryRequest(t, requests[i])) checkMainTx(t, requesters, requests, i+1, shouldComplete) require.Equal(t, completedCount, len(completedTxes)) }