Skip to content

test(beacon_anchor): add 53 unit tests for Beacon v2 envelope system [T3]#6366

Open
waefrebeorn wants to merge 1 commit into
Scottcjn:mainfrom
waefrebeorn:clean/t3-beacon-anchor
Open

test(beacon_anchor): add 53 unit tests for Beacon v2 envelope system [T3]#6366
waefrebeorn wants to merge 1 commit into
Scottcjn:mainfrom
waefrebeorn:clean/t3-beacon-anchor

Conversation

@waefrebeorn
Copy link
Copy Markdown

Clean replacement for #6320. Single commit, no unrelated files, no p2p.py regression.

RTC Wallet: RTC17c0d21f04f6f65c1a85c0aeb5d4a305d57531096

…[T3]

Covers 14 functions across:
- Agent ID derivation from Ed25519 pubkeys (bcn_ prefix)
- Canonical signed field extraction and JSON serialization
- Blake2b payload hashing (deterministic, compact separators)
- Ed25519 signature verification (valid, missing fields, bad hex,
  agent_id mismatch, NACL unavailable, BadSignatureError)
- Envelope storage with full validation pipeline
- Digest computation over un-anchored envelopes
- mark_anchored lifecycle management
- Pagination clamping (int, string, overflow, negative)
- get_recent_envelopes (newest-first, limit, offset)
- init_beacon_table (idempotent, indexes, migration column)
- _ensure_payload_hash_version_column migration

53 tests, all pass. Coverage: 0% to covered.

RTC: rtc17c0d21f04f6f65c1a85c0aeb5d4a305d57531096
@github-actions github-actions Bot added size/XL PR: 500+ lines BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related tests Test suite changes labels May 26, 2026
Copy link
Copy Markdown

@shadow88sky shadow88sky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed PR #6366, focused on node/tests/test_beacon_anchor.py for the Beacon v2 envelope system.

The new suite covers canonical signing/hash helpers, agent id derivation, real Ed25519 verification, signature failure cases, store-time validation, duplicate nonce handling, digest computation, anchoring, pagination normalization, recent-envelope ordering, migration/idempotent table setup, and constants. I specifically checked that the signature path is not only mocked: the valid-signature and store-envelope helpers generate real nacl.signing.SigningKey keys and sign the canonical payload.

The digest/storage tests also exercise fresh payload hash version 2 writes and the mixed-version reporting surface. One limitation is that the mixed-version test only asserts the all-new-write case is not mixed; it does not seed a legacy-v1 row plus a v2 row to assert the positive mixed case. That would be a useful follow-up, but I do not see it as a blocker for this test-only PR.

Validation performed:

  • GitHub PR checks are passing
  • git diff --check $(git merge-base HEAD origin/main)..HEAD passed
  • .venv/bin/python -m py_compile node/beacon_anchor.py node/tests/test_beacon_anchor.py passed
  • PYTHONPATH=. PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/python -m pytest node/tests/test_beacon_anchor.py -q -> 53 passed

I received RTC compensation for this review.

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Thanks for contributing to RustChain! 🦀

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Thanks for contributing to RustChain! 🦀

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Thanks for contributing to RustChain! 🦀

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Thanks for contributing to RustChain! 🦀

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this contribution! 🎉

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this contribution! 🎉

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this contribution! 🎉

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this contribution! The code looks well-structured and follows good practices. Appreciate the effort put into this PR.

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this contribution! The code looks well-structured.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related size/XL PR: 500+ lines tests Test suite changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants