Skip to content

kaiax/vrank, blockchain/types: reject present-but-empty VRank and cou…#989

Open
hyunsooda wants to merge 1 commit into
kaiachain:devfrom
hyunsooda:fix/vrank-header-presence-and-size
Open

kaiax/vrank, blockchain/types: reject present-but-empty VRank and cou…#989
hyunsooda wants to merge 1 commit into
kaiachain:devfrom
hyunsooda:fix/vrank-header-presence-and-size

Conversation

@hyunsooda

@hyunsooda hyunsooda commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Proposed changes

  • Pre-fork VRank guard checked len(header.VRank) > 0, but an explicit empty RLP
    string decodes to a non-nil zero-length slice — passes the length check yet
    re-encodes as a trailing element. Check presence (!= nil) instead.
  • Header.Size() omitted the VRank payload; add len(h.VRank).

Types of changes

  • 🐛 Bug fix
  • ✨ Non-hardfork changes (node upgrade not required)
  • 💥 Hardfork / consensus-breaking changes
  • 🧪 Test improvements
  • 🧰 CI / build tool
  • ♻️ Chore / Refactor / Non-functional changes

Checklist

  • 📖 I have read the CONTRIBUTING GUIDELINES doc
  • 📝 I have signed in the PR comment I have read the CLA Document and I hereby sign the CLA in first time contribute after having read CLA
  • 🟢 Lint and unit tests pass locally with my changes ($ make test)

Related issues

Further comments

…nt it in header size

Before the permissionless fork the VRank header field must be absent. The
guard checked len(VRank) > 0, but an explicitly encoded empty RLP string
decodes to a non-nil zero-length slice: it passes the length check yet is
re-encoded as a trailing element, so nodes carrying the field accept the
header while nodes without it reject the extra element. Check presence
(!= nil) instead.

Separately, Header.Size() omitted the VRank payload, undercounting headers
that carry it. Add len(h.VRank) to the byte size.

Confidence: high
Scope-risk: narrow

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@hyunsooda hyunsooda self-assigned this Jul 20, 2026
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