Skip to content

Commit b92516c

Browse files
pk910pk910
authored
Merge pull request #521 from ethpandaops/pk910/majority-specs
use majority spec values
2 parents 0a52a71 + 3847ce1 commit b92516c

File tree

2 files changed

+119
-54
lines changed

2 files changed

+119
-54
lines changed

‎clients/consensus/chainstate.go‎

Lines changed: 113 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@ import (
1515
)
1616

1717
type ChainState struct {
18-
specMutex sync.RWMutex
19-
specs *ChainSpec
18+
specMutex sync.RWMutex
19+
specs *ChainSpec
20+
clientSpecs map[*Client]map[string]interface{}
2021

2122
genesisMutex sync.Mutex
2223
genesis *v1.Genesis
@@ -33,7 +34,9 @@ type ChainState struct {
3334
}
3435

3536
func newChainState() *ChainState {
36-
return &ChainState{}
37+
return &ChainState{
38+
clientSpecs: make(map[*Client]map[string]interface{}),
39+
}
3740
}
3841

3942
func (cs *ChainState) setGenesis(genesis *v1.Genesis) error {
@@ -55,69 +58,133 @@ func (cs *ChainState) setGenesis(genesis *v1.Genesis) error {
5558
return nil
5659
}
5760

58-
func (cs *ChainState) setClientSpecs(specValues map[string]interface{}) (error, error) {
61+
func (cs *ChainState) updateClientSpecs(client *Client, specValues map[string]interface{}) error {
5962
cs.specMutex.Lock()
6063
defer cs.specMutex.Unlock()
6164

62-
specs := cs.specs
63-
if specs == nil {
64-
specs = &ChainSpec{}
65-
} else {
66-
specs = specs.Clone()
67-
}
65+
// Store this client's specs
66+
cs.clientSpecs[client] = specValues
6867

69-
err := specs.ParseAdditive(specValues)
68+
// Compute the majority-based global specs
69+
majoritySpecs, err := cs.computeMajoritySpecs()
7070
if err != nil {
71-
return nil, err
71+
return err
7272
}
7373

74-
var warning error
74+
cs.specs = majoritySpecs
7575

76-
if cs.specs != nil {
77-
mismatches, err := cs.specs.CheckMismatch(specs)
78-
if err != nil {
79-
return nil, err
76+
// Update warnings for all clients against the new majority spec
77+
if majoritySpecs != nil {
78+
for c, specs := range cs.clientSpecs {
79+
warnings := cs.checkClientSpecWarnings(majoritySpecs, specs)
80+
c.specWarnings = warnings
81+
c.hasBadSpecs = len(warnings) > 0
8082
}
81-
if len(mismatches) > 0 {
82-
isError := false
83-
mismatchesStr := []string{}
84-
for _, mismatch := range mismatches {
85-
if mismatch.Severity != "ignore" {
86-
mismatchesStr = append(mismatchesStr, mismatch.Name)
87-
}
88-
if mismatch.Severity == "error" {
89-
isError = true
90-
}
91-
}
92-
if isError {
93-
return nil, fmt.Errorf("spec mismatch: %v", strings.Join(mismatchesStr, ", "))
94-
} else {
95-
warning = fmt.Errorf("spec mismatch: %v", strings.Join(mismatchesStr, ", "))
83+
}
84+
85+
return nil
86+
}
87+
88+
// checkClientSpecWarnings compares a client's specs against the majority and returns any warnings.
89+
// Must be called with specMutex held.
90+
func (cs *ChainState) checkClientSpecWarnings(majoritySpecs *ChainSpec, specValues map[string]interface{}) []string {
91+
warnings := []string{}
92+
93+
clientSpec := &ChainSpec{}
94+
err := clientSpec.ParseAdditive(specValues)
95+
if err != nil {
96+
return []string{fmt.Sprintf("failed to parse specs: %v", err)}
97+
}
98+
99+
// Check for mismatches: client has different values than majority
100+
mismatches, err := majoritySpecs.CheckMismatch(clientSpec)
101+
if err != nil {
102+
return []string{fmt.Sprintf("failed to check mismatches: %v", err)}
103+
}
104+
105+
if len(mismatches) > 0 {
106+
mismatchesStr := []string{}
107+
for _, mismatch := range mismatches {
108+
if mismatch.Severity != "ignore" {
109+
mismatchesStr = append(mismatchesStr, mismatch.Name)
96110
}
97111
}
112+
if len(mismatchesStr) > 0 {
113+
warnings = append(warnings, fmt.Sprintf("spec mismatch with majority: %v", strings.Join(mismatchesStr, ", ")))
114+
}
115+
}
98116

99-
newSpecs := &ChainSpec{}
100-
err = newSpecs.ParseAdditive(specValues)
101-
if err != nil {
102-
return nil, err
117+
// Check for missing specs: client doesn't have values that majority has
118+
// (by reversing the check - client's zero values are ignored, so we find majority values client doesn't have)
119+
mismatches, err = clientSpec.CheckMismatch(majoritySpecs)
120+
if err != nil {
121+
return warnings
122+
}
123+
if len(mismatches) > 0 {
124+
missingStr := []string{}
125+
for _, mismatch := range mismatches {
126+
missingStr = append(missingStr, mismatch.Name)
127+
}
128+
if len(missingStr) > 0 {
129+
warnings = append(warnings, fmt.Sprintf("spec missing: %v", strings.Join(missingStr, ", ")))
103130
}
131+
}
132+
133+
return warnings
134+
}
135+
136+
// computeMajoritySpecs computes the global spec by taking the majority value for each spec setting.
137+
// Must be called with specMutex held.
138+
func (cs *ChainState) computeMajoritySpecs() (*ChainSpec, error) {
139+
if len(cs.clientSpecs) == 0 {
140+
return nil, nil
141+
}
142+
143+
// Build a map of spec key -> value -> count
144+
valueCounts := make(map[string]map[string]int)
145+
valueStore := make(map[string]map[string]interface{})
146+
147+
for _, clientSpec := range cs.clientSpecs {
148+
for key, value := range clientSpec {
149+
if valueCounts[key] == nil {
150+
valueCounts[key] = make(map[string]int)
151+
valueStore[key] = make(map[string]interface{})
152+
}
104153

105-
mismatches, err = cs.specs.CheckMismatch(newSpecs)
106-
if err != nil {
107-
return nil, err
154+
// Convert value to string for counting (handles different types)
155+
valueStr := fmt.Sprintf("%v", value)
156+
valueCounts[key][valueStr]++
157+
valueStore[key][valueStr] = value
108158
}
109-
if len(mismatches) > 0 {
110-
mismatchesStr := []string{}
111-
for _, mismatch := range mismatches {
112-
mismatchesStr = append(mismatchesStr, mismatch.Name)
159+
}
160+
161+
// Build the majority spec values
162+
majorityValues := make(map[string]interface{})
163+
164+
for key, counts := range valueCounts {
165+
// Find the value with the highest count
166+
var maxCount int
167+
var maxValueStr string
168+
169+
for valueStr, count := range counts {
170+
if count > maxCount {
171+
maxCount = count
172+
maxValueStr = valueStr
113173
}
114-
warning = fmt.Errorf("spec missing: %v", strings.Join(mismatchesStr, ", "))
115174
}
175+
176+
// Use the actual value (not the string representation)
177+
majorityValues[key] = valueStore[key][maxValueStr]
116178
}
117179

118-
cs.specs = specs
180+
// Parse into ChainSpec
181+
majoritySpec := &ChainSpec{}
182+
err := majoritySpec.ParseAdditive(majorityValues)
183+
if err != nil {
184+
return nil, err
185+
}
119186

120-
return warning, nil
187+
return majoritySpec, nil
121188
}
122189

123190
func (cs *ChainState) initWallclock() {

‎clients/consensus/clientlogic.go‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -84,21 +84,19 @@ func (client *Client) checkClient() error {
8484
return fmt.Errorf("error while fetching specs: %v", err)
8585
}
8686

87-
warning, err := client.pool.chainState.setClientSpecs(specs)
87+
err = client.pool.chainState.updateClientSpecs(client, specs)
8888
client.specs = specs
8989

9090
if err != nil {
9191
client.hasBadSpecs = true
9292
return fmt.Errorf("invalid chain specs: %v", err)
9393
}
9494

95-
if warning != nil {
96-
client.logger.Warnf("incomplete chain specs: %v", warning)
97-
client.specWarnings = []string{warning.Error()}
98-
client.hasBadSpecs = true
99-
} else {
100-
client.specWarnings = nil
101-
client.hasBadSpecs = false
95+
// Log warnings if any were set by updateClientSpecs
96+
if len(client.specWarnings) > 0 {
97+
for _, warning := range client.specWarnings {
98+
client.logger.Warnf("chain spec issue: %v", warning)
99+
}
102100
}
103101

104102
// init wallclock

0 commit comments

Comments
 (0)