primitives%fix!: refuse governance object signatures over 96 bytes by PastaPastaPasta · Pull Request #126 · dashpay/base-sdk

PastaPastaPasta · 2026-11-06T06:51:59Z

Motivation

The reference implementation v24.0.0 reads a governance object's signature as a vector of at most 96 bytes, and refuses a longer length before reading the bytes (dashpay/dash#7831). dash-primitives reads it up to MAX_SER_SIZE. This pull request bounds it at 96, using impl_vec!'s field: limit from #119. #119 named this field as a follow-up.

Additional Information

Description	Notes	Reference implementation	Corpus
Bound `GovObject::sig` at 96 bytes	`GovObject::MAX_SIGNATURE_SIZE` is 96. `GovObject` declares itself with `impl_new!` and takes its codec from `codec_base!`'s `$ty, $max, { fields }` form, which now passes a `field: limit` through to `impl_vec!`. The signature count is checked before any signature byte is read, and a longer count is `DecodeError::CompactSizeExceedsLimit`. The other fields, `data` included, read as before. Encoding is unchanged.	bound, read, data	`govobj.json5`, `proposals.json5` and `triggers.json5`, unchanged

Changelog

The crate is unreleased, so this entry is kept here instead of in pkgs/primitives/CHANGELOG.md.

Added

GovObject::MAX_SIGNATURE_SIZE, the most signature bytes a governance object carries (96).
codec_base! reads a field written field: limit with at most limit entries, in its $ty, $max, { fields } form.

Changed

GovObject refuses a signature longer than GovObject::MAX_SIGNATURE_SIZE when decoding, before reading its bytes.

Breaking Changes

GovObject refuses a signature longer than 96 bytes when decoding, which it read before. dash-p2p-core's govobj message, which carries a GovObject, refuses it too.

How Has This Been Tested?

codec_base_bounds_a_limited_field checks codec_base! itself on a stand-in struct with a field written items: 2: an empty list and a list at the limit decode and encode back to the same bytes, and a count of 3 is refused when only the count is on the wire, so no entry is read first. No test decodes a GovObject with a long signature: its bound is the sig: Self::MAX_SIGNATURE_SIZE entry in that codec_base! form, whose limit this test checks.
Every vector in govobj.json5, proposals.json5 and triggers.json5 still decodes and encodes to the same bytes.

Checklist

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added or updated relevant unit/integration/functional tests
I have made corresponding changes to the documentation
I did not use agentic assistance (i.e. LLMs, AI) except as disclosed and can answer questions,
comments and concerns without said assistance. I assume authorship of the code submitted.
I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

Agent disclosure

Code, commit messages and this description were generated with Claude Code using Claude Opus 5.5, running autonomously as an agent on behalf of the author. The change was reviewed by a Claude Code agent using Opus 5.5.

🤖 Generated with Claude Code

coderabbitai · 2026-11-06T06:52:08Z

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

New Features
- Encoding declarations now support optional limits on individual fields, while existing declarations remain supported.
- Governance objects now expose a 96-byte maximum signature size.
Bug Fixes
- Decoding rejects governance-object signatures longer than 96 bytes, including when the oversized signature data is missing.

The codec-only codec_base! form now accepts optional per-field limits. GovObject uses this support to cap signatures at 96 bytes and exposes the limit through MAX_SIGNATURE_SIZE. Tests cover decoding signatures at the boundary and rejection of a declared size above the limit, whether or not the signature bytes are present.

Merge Risk: Minimal · up to 74b13

No actionable issue is established; the change is mergeable after normal checks.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title identifies the 96-byte governance object signature limit.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The pull request description directly explains the 96-byte GovObject signature limit, codec_base! changes, decoding behavior, tests, and compatibility impact. It matches the changeset.

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

Source mode: Auto

Repository: dashpay/base-sdk

Branch: pasta/gov-object-sig

File: .coderabbit.yml

Commit: 74b1364

Layers: Repository YAML

Review profile: CHILL

Plan: Advanced

Run ID: `5228ac45-852d-4a0f-8f11-77d4ed88f249`

Commits

Reviewing files that changed from the base of the PR and between `90952f8` and `74b1364`.

Files selected for processing: 3

`pkgs/primitives/CHANGELOG.md`

`pkgs/primitives/src/codec.rs`

`pkgs/primitives/src/gov/object.rs`

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

thepastaclaw · 2026-11-06T06:56:47Z

✅ Final review complete — no blockers (commit 8ad5a62) · triage: low

codecov · 2026-11-06T07:00:12Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

thepastaclaw

Final validation — Phase 1 + Phase 2

Verified the complete diff at 74b1364 and found no actionable defects. The signature limit matches the pinned Dash Core implementation and is enforced before signature allocation or byte reads, while encoding, field order, construction, and hashing remain unchanged. Validation was static only; the supplied CI snapshot and current checks show successful applicable build, test, lint, and coverage checks, with none failing or pending.

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

Triage: low by gpt-6.1-sol (effort low) — This is a small, contained decoding-bound fix with focused boundary tests and a straightforward macro passthrough, not a large or intricate change despite touching peer-facing deserialization.
Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer, glm-5.3-flash — rust-quality (completed, effort high); agent phase1-reviewer
Phase 1 model: glm-5.3-flash — zai quota: 5h 99% left, weekly 44% left; passed over gemini-3.8-flash-high (antigravity quota lookup failed)
Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort medium); agent phase2-reviewer

github-actions · 2026-11-06T23:54:26Z

Note

This pull request has no conflicts! 🎊 🎉 🎊

`codec_base!` takes `field: limit` in its codec form. `GovObject` reads at most `GovObject::MAX_SIGNATURE_SIZE` signature bytes and refuses a longer count before reading any of them.

kwvg · 2026-11-11T07:09:47Z

-codec_base! {
+impl_new! {
 /// A governance object as serialized on the wire.
 ///


We can probably drop the below segment and shorten the description to "Governance object", the expanded name was an artefact a noisier iteration of this crate

```text

Done in 49d3978: the doc is now "Governance object.".

🤖 Posted autonomously by Claude on behalf of pasta.

kwvg · 2026-11-11T07:10:45Z

+  #[rstest]
+  #[case::with_the_bytes(true)]
+  #[case::without_the_bytes(false)]
+  fn refuses_a_sig_one_over_the_bound(#[case] with_bytes: bool) {


Tautological test, it's effectively a check on codec_base! itself, prefer if there is a general check for codec_base! itself against a dummy struct and then all three cases can be validated and, transiently covering all invocations of the macro

Done in c10390b: codec_base_bounds_a_limited_field in codec.rs checks the field: limit form on a stand-in struct (empty, at the limit, a count over it with no entries after it), and the GovObject-level tests are gone.

🤖 Posted autonomously by Claude on behalf of pasta.

kwvg · 2026-11-11T07:11:07Z


 - Initial release.
+
+### Added


This should be in the PR description, unreleased crates don't get changelog updates.

Done in 2491b5d, and in #121 with 0aeeeb5. Both entries are now in the PR descriptions.

🤖 Posted autonomously by Claude on behalf of pasta.

Drop the wire layout block, which repeated the fields in order, and describe the type as a governance object.

One test on a stand-in struct covers an empty list, a list at the limit and a count over it, in place of the governance object's tests.

The crate is unreleased, so the change is described in the pull request instead.

The base form doc names the two macros built on it, as on develop, and adds only the field limit.

thepastaclaw

Re-review — Final validation — Phase 1 + Phase 2

At head 8ad5a62, the signature bound matches the pinned Dash Core implementation and rejects oversized counts before signature allocation or decoding, without changing encoding or hashing; no actionable defects were verified. Review was static only; current CI shows successful lint, MSRV build/test, stable-platform, and cross-compilation checks. The failed “Tag pull requests” and “Comment on pull requests” checks are administrative workflows unrelated to code correctness; coverage and review checks remain pending.

🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

Triage: low by gpt-6.1-sol (effort low) — The change is small and contained, forwarding an existing field-limit mechanism through codec_base! and applying a 96-byte signature decoding bound, without the size or intricacy required for critical.
Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer, glm-5.3-flash — rust-quality (completed, effort high); agent phase1-reviewer
Phase 1 model: glm-5.3-flash — zai quota: 5h 77% left, weekly 76% left; passed over gemini-3.8-flash-high (antigravity quota lookup failed)
Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort medium); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.

No unresolved findings remain from the prior review on this head.

PastaPastaPasta added this to the 0.3 milestone Nov 6, 2026

PastaPastaPasta requested a review from kwvg November 6, 2026 06:52

thepastaclaw approved these changes Nov 6, 2026

View reviewed changes

PastaPastaPasta changed the base branch from pasta/p2p-v1-framing to develop November 6, 2026 22:06

github-actions Bot mentioned this pull request Nov 6, 2026

primitives%feat: add payload sign hashes, read signatures of any length #121

Open

4 of 6 tasks

github-actions Bot added the status:behind This pull request can be rebased on the base branch. label Nov 6, 2026

PastaPastaPasta added 2 commits November 7, 2026 02:27

primitives%fix!: refuse governance object signatures over 96 bytes

7ae88ae

`codec_base!` takes `field: limit` in its codec form. `GovObject` reads at most `GovObject::MAX_SIGNATURE_SIZE` signature bytes and refuses a longer count before reading any of them.

primitives%doc: update changelog for the signature bound

7b552ac

PastaPastaPasta force-pushed the pasta/gov-object-sig branch from 74b1364 to 7b552ac Compare November 7, 2026 02:34

github-actions Bot removed the status:behind This pull request can be rebased on the base branch. label Nov 7, 2026

kwvg requested changes Nov 11, 2026

View reviewed changes

PastaPastaPasta added 4 commits November 11, 2026 07:16

primitives%doc: shorten the governance object doc

49d3978

Drop the wire layout block, which repeated the fields in order, and describe the type as a governance object.

primitives%test: test the field limit of codec_base! on its own

c10390b

One test on a stand-in struct covers an empty list, a list at the limit and a count over it, in place of the governance object's tests.

primitives%doc: drop the changelog entry for the signature bound

2491b5d

The crate is unreleased, so the change is described in the pull request instead.

primitives%doc: keep develop's callers in the codec_base! doc

8ad5a62

The base form doc names the two macros built on it, as on develop, and adds only the field limit.

PastaPastaPasta requested a review from kwvg November 11, 2026 15:55

thepastaclaw approved these changes Nov 11, 2026

View reviewed changes

Conversation

PastaPastaPasta commented Nov 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Additional Information

Changelog

Added

Changed

Breaking Changes

How Has This Been Tested?

Checklist

Agent disclosure

Uh oh!

coderabbitai Bot commented Nov 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

thepastaclaw commented Nov 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Nov 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Final validation — Phase 1 + Phase 2

Uh oh!

github-actions Bot commented Nov 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kwvg Nov 11, 2026

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta Nov 11, 2026

Choose a reason for hiding this comment

Uh oh!

kwvg Nov 11, 2026

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta Nov 11, 2026

Choose a reason for hiding this comment

Uh oh!

kwvg Nov 11, 2026

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta Nov 11, 2026

Choose a reason for hiding this comment

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Re-review — Final validation — Phase 1 + Phase 2

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

PastaPastaPasta commented Nov 6, 2026 •

edited

Loading

coderabbitai Bot commented Nov 6, 2026 •

edited

Loading

thepastaclaw commented Nov 6, 2026 •

edited

Loading

codecov Bot commented Nov 6, 2026 •

edited

Loading

github-actions Bot commented Nov 6, 2026 •

edited

Loading