FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat(consensus): bounded-deviation check for preferHighestValueFor by andreclaro · Pull Request #1069 · erpc/erpc · GitHub

/ erpc Public

feat(consensus): bounded-deviation check for preferHighestValueFor - #1069

Open
andreclaro wants to merge 2 commits into
erpc:mainfrom
andreclaro:feat/consensus-bounded-deviation
Open

feat(consensus): bounded-deviation check for preferHighestValueFor#1069
andreclaro wants to merge 2 commits into
erpc:mainfrom
andreclaro:feat/consensus-bounded-deviation

Conversation

Copy link
Copy Markdown
Contributor

Problem

preferHighestValueFor picks the numerically highest value across upstreams for configured methods/fields, gated only by agreementThreshold (how many upstreams must report the same value). It has no check on how far a value sits from the rest of the pack: one upstream returning a wildly larger number (stale cache, corrupted state, bug) can win outright as long as it independently satisfies agreementThreshold — even with agreementThreshold: 1, unconditionally.

Change

Adds an opt-in, per-method preferHighestValueForMaxDeviationPct config. Before picking the highest value among threshold-qualifying candidates, candidates whose primary field deviates from the median of the other qualifying candidates by more than the configured percent are filtered out first.

consensus:
  preferHighestValueFor:
    eth_getTransactionCount: ["result"]
  preferHighestValueForMaxDeviationPct:
    eth_getTransactionCount: 50
  • Keyed per-method (same shape as preferHighestValueFor) since natural spread differs by method — a nonce should track tightly across synced nodes, a fee estimate may legitimately vary more.
  • Unset (default) is a complete no-op — existing configs behave byte-for-byte identically. Covered by a regression test.
  • Median/deviation math uses big.Int/big.Rat, not float64, to avoid precision loss on large values (balances, wei amounts).
  • If every qualifying candidate is filtered out, falls through to the existing ErrConsensusDispute path — no new error type.
  • Rejections are logged at debug level (method, rejected value, median, configured bound) for observability.
  • Validation: a configured deviation percentage requires a corresponding preferHighestValueFor entry for the same method, and rejects negative percentages.

Testing

  • New unit tests for the medianBigInt / withinMaxDeviation helpers (consensus/utils_test.go), covering odd/even-count medians, exact/at-bound/just-outside boundaries, nil/zero-median no-op, and large uint64 values.
  • Extended the existing preferHighestValueFor integration table (erpc/networks_consensus_test.go) with cases for: outlier rejection, all-within-bound (unaffected), all-candidates-rejected (dispute), and an explicit regression case proving default (unset) behavior is unchanged.
  • go build ./... and the full consensus/common packages plus the relevant erpc package tests pass locally.

Scope

This is intentionally the smaller of two approaches considered for guarding preferHighestValueFor against outliers: it keeps the existing design of trusting multiple upstreams' own reported values (rather than collapsing to a single source of truth) and makes no client-facing response-contract change. A distinct "flag without rejecting" mode was considered and dropped — flagging with no enforcement had no forcing use case, and rejection is already observable via the debug log line.

preferHighestValueFor picks the numerically highest value across
upstreams for configured methods/fields, gated only by
agreementThreshold. A single upstream returning an outlier (stale
cache, corrupted state, bug) can still win outright as long as it
independently satisfies the threshold.

Add an opt-in, per-method PreferHighestValueForMaxDeviationPct config
that filters out candidate values straying more than N percent from
the median of all threshold-qualifying candidates before picking the
highest survivor. Unset (default) keeps existing behavior byte-for-byte
unchanged.
andreclaro marked this pull request as ready for review August 12, 2026 16:01

subweave Bot commented Aug 12, 2026
edited
Loading

Copy link
Copy Markdown

This looks like a really clever way to handle outliers! 🤓
Explore here →

Carefully crafted by Subweave · 🧶 used ~590k LLM tokens

…n-finite pct, document config

Address review findings on the bounded-deviation check:

- rules.go: the median for preferHighestValueForMaxDeviationPct was
  computed from one value per distinct candidate group, ignoring how
  many upstreams voted for it. For [5, 5, 500] with threshold=1, that
  took the median of [5, 500] and rejected both as outliers instead of
  keeping the 2-vote majority. Weight the median by each candidate's
  agreement count instead.
- validation.go: reject NaN/+-Inf for preferHighestValueForMaxDeviationPct.
  NaN silently disabled filtering (NaN comparisons are always false);
  +Inf reached big.Rat.SetFloat64, which returns nil for non-finite
  input, causing a nil-pointer panic in the analyzer.
- docs: document preferHighestValueForMaxDeviationPct in the consensus
  config reference.
- tests: regression case for the majority-vs-outlier median weighting,
  and validation tests for NaN/Inf/negative/missing-pair rejection.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant


Back | FazBrowse Home | New Git URL