Skip to content

Commit 595348f

Browse files
theflyingcodrtheflyingcodr
authored
Merge pull request #8 from libsv/fix/pubKeyBytes_race
Data race fix - pubKeyBytes
2 parents ccbb89a + 0cb734a commit 595348f

File tree

4 files changed

+273
-8
lines changed

4 files changed

+273
-8
lines changed

‎.github/mergify.yml

+241
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,241 @@
1+
pull_request_rules:
2+
3+
# ===============================================================================
4+
# DEPENDABOT
5+
# ===============================================================================
6+
7+
- name: Automatic Merge for Dependabot Minor Version Pull Requests
8+
conditions:
9+
- -draft
10+
- author~=^dependabot(|-preview)\[bot\]$
11+
- check-success='build (1.16.x, ubuntu-latest)'
12+
- check-success='build (1.17.x, ubuntu-latest)'
13+
- check-success='build (1.16.x, macos-latest)'
14+
- check-success='build (1.17.x, macos-latest)'
15+
- check-success='lint (1.16.x, ubuntu-latest)'
16+
- check-success='lint (1.16.x, macos-latest)'
17+
- check-success='lint (1.17.x, macos-latest)'
18+
- check-success='Analyze (go)'
19+
- title~=^Bump [^\s]+ from ([\d]+)\..+ to \1\.
20+
actions:
21+
review:
22+
type: APPROVE
23+
message: Automatically approving dependabot pull request
24+
merge:
25+
method: squash
26+
- name: Alert on major version detection
27+
conditions:
28+
- author~=^dependabot(|-preview)\[bot\]$
29+
- check-success='build (1.16.x, ubuntu-latest)'
30+
- check-success='build (1.17.x, ubuntu-latest)'
31+
- check-success='build (1.16.x, macos-latest)'
32+
- check-success='build (1.17.x, macos-latest)'
33+
- check-success='lint (1.16.x, ubuntu-latest)'
34+
- check-success='lint (1.16.x, macos-latest)'
35+
- check-success='lint (1.17.x, macos-latest)'
36+
- check-success='Analyze (go)'
37+
- -title~=^Bump [^\s]+ from ([\d]+)\..+ to \1\.
38+
actions:
39+
comment:
40+
message: "⚠️ @theflyingcodr @jadwahab: this is a major version bump and requires your attention"
41+
42+
43+
# ===============================================================================
44+
# AUTOMATIC MERGE (APPROVALS)
45+
# ===============================================================================
46+
47+
- name: Automatic Merge ⬇️ on Approval ✔
48+
conditions:
49+
- "#approved-reviews-by>=1"
50+
- check-success='build (1.16.x, ubuntu-latest)'
51+
- check-success='build (1.17.x, ubuntu-latest)'
52+
- check-success='build (1.16.x, macos-latest)'
53+
- check-success='build (1.17.x, macos-latest)'
54+
- check-success='lint (1.16.x, ubuntu-latest)'
55+
- check-success='lint (1.16.x, macos-latest)'
56+
- check-success='lint (1.17.x, macos-latest)'
57+
- check-success='Analyze (go)'
58+
- label!=work-in-progress
59+
- -draft
60+
actions:
61+
merge:
62+
method: squash
63+
64+
# ===============================================================================
65+
# AUTHOR
66+
# ===============================================================================
67+
68+
- name: Auto-Assign Author
69+
conditions:
70+
- "#assignee=0"
71+
actions:
72+
assign:
73+
add_users:
74+
- "{{author}}"
75+
76+
# ===============================================================================
77+
# ALERTS
78+
# ===============================================================================
79+
80+
- name: Notify on merge
81+
conditions:
82+
- merged
83+
- label=automerge
84+
actions:
85+
comment:
86+
message: "✅ @{{author}}: **{{title}}** has been merged successfully."
87+
- name: Alert on merge conflict
88+
conditions:
89+
- conflict
90+
- label=automerge
91+
actions:
92+
comment:
93+
message: "🆘 @{{author}}: `{{head}}` has conflicts with `{{base}}` that must be resolved."
94+
label:
95+
add:
96+
- conflict
97+
- name: Alert on tests failure for automerge
98+
conditions:
99+
- label=automerge
100+
- status-failure=commit
101+
actions:
102+
comment:
103+
message: "🆘 @{{author}}: unable to merge due to CI failure."
104+
105+
- name: remove conflict label if not needed
106+
conditions:
107+
- -conflict
108+
actions:
109+
label:
110+
remove:
111+
- conflict
112+
113+
# ===============================================================================
114+
# LABELS
115+
# ===============================================================================
116+
# Automatically add labels when PRs match certain patterns
117+
#
118+
# NOTE:
119+
# - single quotes for regex to avoid accidental escapes
120+
# - Mergify leverages Python regular expressions to match rules.
121+
#
122+
# Semantic commit messages
123+
# - chore: updating grunt tasks etc.; no production code change
124+
# - docs: changes to the documentation
125+
# - feat: feature or story
126+
# - enhancement: an improvement to an existing feature
127+
# - feat: new feature for the user, not a new feature for build script
128+
# - fix: bug fix for the user, not a fix to a build script
129+
# - idea: general idea or suggestion
130+
# - test: test related changes
131+
# ===============================================================================
132+
133+
- name: Hotfix label
134+
conditions:
135+
- "head~=(?i)^hotfix" # if the PR branch starts with hotfix/
136+
actions:
137+
label:
138+
add: ["hot-fix"]
139+
- name: Bug / Fix label
140+
conditions:
141+
- "head~=(?i)^(bug)?fix" # if the PR branch starts with (bug)?fix/
142+
actions:
143+
label:
144+
add: [ "bug-P3" ]
145+
- name: Documentation label
146+
conditions:
147+
- "head~=(?i)^docs" # if the PR branch starts with docs/
148+
actions:
149+
label:
150+
add: [ "documentation" ]
151+
- name: Feature label
152+
conditions:
153+
- "head~=(?i)^feat(ure)?" # if the PR branch starts with feat(ure)?/
154+
actions:
155+
label:
156+
add: ["feature"]
157+
- name: Enhancement label
158+
conditions:
159+
- "head~=(?i)^enhancement?" # if the PR branch starts with enhancement/
160+
actions:
161+
label:
162+
add: ["enhancement"]
163+
- name: Chore label
164+
conditions:
165+
- "head~=(?i)^chore" # if the PR branch starts with chore/
166+
actions:
167+
label:
168+
add: ["update"]
169+
- name: Question label
170+
conditions:
171+
- "head~=(?i)^question" # if the PR branch starts with question/
172+
actions:
173+
label:
174+
add: ["question"]
175+
- name: Test label
176+
conditions:
177+
- "head~=(?i)^test" # if the PR branch starts with test/
178+
actions:
179+
label:
180+
add: ["test"]
181+
- name: Idea label
182+
conditions:
183+
- "head~=(?i)^idea" # if the PR branch starts with idea/
184+
actions:
185+
label:
186+
add: ["idea"]
187+
188+
# ===============================================================================
189+
# STALE BRANCHES
190+
# ===============================================================================
191+
192+
- name: Close stale pull request
193+
conditions:
194+
- base=master
195+
- -closed
196+
- updated-at<21 days ago
197+
actions:
198+
close:
199+
message: |
200+
This pull request looks stale. Feel free to reopen it if you think it's a mistake.
201+
label:
202+
add: [ "stale" ]
203+
# ===============================================================================
204+
# BRANCHES
205+
# ===============================================================================
206+
207+
- name: Delete head branch after merge
208+
conditions:
209+
- merged
210+
actions:
211+
delete_head_branch:
212+
213+
#- name: automatic update for PR marked as “Ready-to-Go“
214+
# conditions:
215+
# - -conflict # skip PRs with conflicts
216+
# - -draft # filter-out GH draft PRs
217+
# - label="Ready-to-Go"
218+
# actions:
219+
# update:
220+
221+
# ===============================================================================
222+
# CONVENTION
223+
# ===============================================================================
224+
# https://www.conventionalcommits.org/en/v1.0.0/
225+
# Premium feature only
226+
227+
#- name: Conventional Commit
228+
# conditions:
229+
# - "title~=^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\\(.+\\))?:"
230+
# actions:
231+
# post_check:
232+
# title: |
233+
# {% if check_succeed %}
234+
# Title follows Conventional Commit
235+
# {% else %}
236+
# Title does not follow Conventional Commit
237+
# {% endif %}
238+
# summary: |
239+
# {% if not check_succeed %}
240+
# Your pull request title must follow [Conventional Commit](https://www.conventionalcommits.org/en/v1.0.0/).
241+
# {% endif %}

‎.github/workflows/run-tests.yml

+2-2
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ jobs:
1212
golangci:
1313
strategy:
1414
matrix:
15-
go-version: [1.15.x,1.16.x]
15+
go-version: [1.16.x, 1.17.x]
1616
os: [macos-latest, ubuntu-latest]
1717
name: lint
1818
runs-on: ${{ matrix.os }}
@@ -26,7 +26,7 @@ jobs:
2626
build:
2727
strategy:
2828
matrix:
29-
go-version: [ 1.13.x,1.14.x,1.15.x, 1.16.x ]
29+
go-version: [ 1.16.x, 1.17.x ]
3030
os: [ macos-latest, ubuntu-latest ]
3131
runs-on: ${{ matrix.os }}
3232
steps:

‎bip32/extendedkey.go

+10-6
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"errors"
1818
"fmt"
1919
"math/big"
20+
"sync"
2021

2122
"github.com/libsv/go-bk/base58"
2223
"github.com/libsv/go-bk/bec"
@@ -113,6 +114,7 @@ type ExtendedKey struct {
113114
childNum uint32
114115
depth uint8
115116
isPrivate bool
117+
o sync.Once
116118
}
117119

118120
// NewExtendedKey returns a new instance of an extended key with the given
@@ -151,12 +153,14 @@ func (k *ExtendedKey) pubKeyBytes() []byte {
151153
}
152154

153155
// This is a private extended key, so calculate and memoize the public
154-
// key if needed.
155-
if len(k.pubKey) == 0 {
156-
pkx, pky := bec.S256().ScalarBaseMult(k.key)
157-
pubKey := bec.PublicKey{Curve: bec.S256(), X: pkx, Y: pky}
158-
k.pubKey = pubKey.SerialiseCompressed()
159-
}
156+
// key if needed once and only once.
157+
k.o.Do(func() {
158+
if len(k.pubKey) == 0 {
159+
pkx, pky := bec.S256().ScalarBaseMult(k.key)
160+
pubKey := bec.PublicKey{Curve: bec.S256(), X: pkx, Y: pky}
161+
k.pubKey = pubKey.SerialiseCompressed()
162+
}
163+
})
160164

161165
return k.pubKey
162166
}

‎bip32/extendedkey_test.go

+20
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@ import (
1212
"bytes"
1313
"encoding/hex"
1414
"errors"
15+
"fmt"
1516
"math"
1617
"reflect"
18+
"sync"
1719
"testing"
1820

1921
"github.com/libsv/go-bk/chaincfg"
@@ -922,3 +924,21 @@ func TestMaximumDepth(t *testing.T) {
922924
t.Fatal("Child: deriving 256th key should not succeed")
923925
}
924926
}
927+
928+
func Test_pubKeyBytes_race(t *testing.T) {
929+
t.Parallel()
930+
extKey, err := NewMaster([]byte(`abcd1234abcd1234abcd1234abcd1234`), &chaincfg.MainNet)
931+
if err != nil {
932+
t.Fatalf("NewMaster: unexpected error: %v", err)
933+
}
934+
wg := sync.WaitGroup{}
935+
for i := 0; i < 10000; i++ {
936+
wg.Add(1)
937+
go func(c int) {
938+
defer wg.Done()
939+
extKey.pubKeyBytes()
940+
extKey.DeriveChildFromPath(fmt.Sprintf("0/0/%d", c))
941+
}(i)
942+
}
943+
wg.Wait()
944+
}

0 commit comments

Comments
 (0)