Skip to content

Commit

Permalink
fix race for global vars (#12512)
Browse files Browse the repository at this point in the history
Co-authored-by: shota.silagadze <[email protected]>
  • Loading branch information
shotasilagadze and shotasilagadzetaal authored Oct 29, 2024
1 parent 2cbd4ad commit 18cc7fb
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 7 deletions.
26 changes: 20 additions & 6 deletions cl/phase1/network/services/attestation_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ func (t *attestationTestSuite) SetupTest() {
netConfig := &clparams.NetworkConfig{}
emitters := beaconevents.NewEventEmitter()
computeSigningRoot = func(obj ssz.HashableSSZ, domain []byte) ([32]byte, error) { return [32]byte{}, nil }
batchCheckInterval = 1 * time.Millisecond
batchSignatureVerifier := NewBatchSignatureVerifier(context.TODO(), nil)
go batchSignatureVerifier.Start()
ctx, cn := context.WithCancel(context.Background())
Expand All @@ -100,9 +99,10 @@ func (t *attestationTestSuite) TestAttestationProcessMessage() {
msg *solid.Attestation
}
tests := []struct {
name string
mock func()
args args
name string
wantErr bool
mock func()
args args
}{
{
name: "Test attestation with committee index out of range",
Expand All @@ -118,6 +118,7 @@ func (t *attestationTestSuite) TestAttestationProcessMessage() {
subnet: nil,
msg: att,
},
wantErr: true,
},
{
name: "Test attestation with wrong subnet",
Expand All @@ -136,6 +137,7 @@ func (t *attestationTestSuite) TestAttestationProcessMessage() {
subnet: uint64Ptr(1),
msg: att,
},
wantErr: true,
},
{
name: "Test attestation with wrong slot (current_slot < slot)",
Expand All @@ -155,6 +157,7 @@ func (t *attestationTestSuite) TestAttestationProcessMessage() {
subnet: uint64Ptr(1),
msg: att,
},
wantErr: true,
},
{
name: "Attestation is aggregated",
Expand All @@ -178,6 +181,7 @@ func (t *attestationTestSuite) TestAttestationProcessMessage() {
Signature: [96]byte{0, 1, 2, 3, 4, 5},
},
},
wantErr: true,
},
{
name: "Attestation is empty",
Expand All @@ -201,6 +205,7 @@ func (t *attestationTestSuite) TestAttestationProcessMessage() {
Signature: [96]byte{0, 1, 2, 3, 4, 5},
},
},
wantErr: true,
},
{
name: "invalid signature",
Expand All @@ -225,6 +230,7 @@ func (t *attestationTestSuite) TestAttestationProcessMessage() {
subnet: uint64Ptr(1),
msg: att,
},
wantErr: true,
},
{
name: "block header not found",
Expand All @@ -249,6 +255,7 @@ func (t *attestationTestSuite) TestAttestationProcessMessage() {
subnet: uint64Ptr(1),
msg: att,
},
wantErr: true,
},
{
name: "invalid target block",
Expand Down Expand Up @@ -276,6 +283,7 @@ func (t *attestationTestSuite) TestAttestationProcessMessage() {
subnet: uint64Ptr(1),
msg: att,
},
wantErr: true,
},
{
name: "invalid finality checkpoint",
Expand Down Expand Up @@ -309,6 +317,7 @@ func (t *attestationTestSuite) TestAttestationProcessMessage() {
subnet: uint64Ptr(1),
msg: att,
},
wantErr: true,
},
{
name: "success",
Expand Down Expand Up @@ -355,9 +364,14 @@ func (t *attestationTestSuite) TestAttestationProcessMessage() {
log.Printf("test case: %s", tt.name)
t.SetupTest()
tt.mock()
err := t.attService.ProcessMessage(tt.args.ctx, tt.args.subnet, &AttestationWithGossipData{Attestation: tt.args.msg, GossipData: nil})
err := t.attService.ProcessMessage(tt.args.ctx, tt.args.subnet, &AttestationWithGossipData{Attestation: tt.args.msg, GossipData: nil, ImmediateProcess: true})
time.Sleep(time.Millisecond * 60)
t.Require().Error(err, ErrIgnore)
if tt.wantErr {
t.Require().Error(err)
} else {
t.Require().NoError(err)
}

t.True(t.gomockCtrl.Satisfied())
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ func (t *blsToExecutionChangeTestSuite) SetupTest() {
t.emitters = beaconevents.NewEventEmitter()
t.beaconCfg = &clparams.BeaconChainConfig{}
batchSignatureVerifier := NewBatchSignatureVerifier(context.TODO(), nil)
batchCheckInterval = 1 * time.Millisecond
go batchSignatureVerifier.Start()
t.service = NewBLSToExecutionChangeService(*t.operationsPool, t.emitters, t.syncedData, t.beaconCfg, batchSignatureVerifier)
// mock global functions
Expand Down
2 changes: 2 additions & 0 deletions cl/sentinel/gossip.go
Original file line number Diff line number Diff line change
Expand Up @@ -513,9 +513,11 @@ func (g *GossipManager) Start(ctx context.Context) {
logArgs := []interface{}{}
g.subscriptions.Range(func(key, value any) bool {
sub := value.(*GossipSubscription)
sub.lock.Lock()
if sub.topic != nil {
logArgs = append(logArgs, sub.topic.String(), sub.subscribed.Load())
}
sub.lock.Unlock()
return true
})
log.Trace("[Gossip] Subscriptions", "subscriptions", logArgs)
Expand Down
2 changes: 2 additions & 0 deletions cl/sentinel/sentinel.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,8 @@ func (s *Sentinel) isPeerUsefulForAnySubnet(node *enode.Node) bool {

s.subManager.subscriptions.Range(func(key, value any) bool {
sub := value.(*GossipSubscription)
sub.lock.Lock()
defer sub.lock.Unlock()
if sub.sub == nil {
return true
}
Expand Down

0 comments on commit 18cc7fb

Please sign in to comment.