chore: derive an agent wallet's address from its rules#1387
Conversation
📝 WalkthroughWalkthroughThis PR introduces two deterministic cryptographic precompile functions ( ChangesMAA Precompile Implementation
Sequence DiagramssequenceDiagram
participant Handler as PrecompileHandler
participant DeriveLogic as deriveMAAAddress
participant Keccak256 as Keccak-256
Handler->>DeriveLogic: restricted, unrestricted, rulesHash, salt
DeriveLogic->>DeriveLogic: Validate lengths (20, 20, 32 bytes)
DeriveLogic->>DeriveLogic: Concatenate version + inputs + salt
DeriveLogic->>Keccak256: Hash address preimage
Keccak256->>DeriveLogic: 32-byte digest
DeriveLogic->>Handler: Return last 20 bytes as address
sequenceDiagram
participant Handler as PrecompileHandler
participant ComputeLogic as computeRulesHash
participant Normalize as Type Normalizers
participant Deduplicate as Deduplication & Sort
participant Keccak256 as Keccak-256
Handler->>ComputeLogic: fee_mode, fee_bps, fee_flat, bridge, namespaces, actions, body_hashes
ComputeLogic->>Normalize: Coerce TEXT/TEXT[] inputs to native types
Normalize->>ComputeLogic: Normalized values
ComputeLogic->>ComputeLogic: Validate constraints (fee_mode, fee ranges, lengths match)
ComputeLogic->>Deduplicate: Allow-list entries with (namespace, action, bodyHash)
Deduplicate->>Deduplicate: Deduplicate by (namespace, action) pair
Deduplicate->>Deduplicate: Sort bytewise by namespace then action
Deduplicate->>ComputeLogic: Canonical sorted entries
ComputeLogic->>ComputeLogic: Build rules preimage with version, fees, bridge, entries
ComputeLogic->>Keccak256: Hash rules preimage
Keccak256->>ComputeLogic: 32-byte digest
ComputeLogic->>Handler: Return rules hash
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Time Submission Status
You can submit time with the command. Example: See available commands to help comply with our Guidelines. |
|
@holdex pr submit-time 4h |
|
@coderabbitai approve |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
extensions/tn_utils/maa_test.go (3)
62-94: 💤 Low valueOptional: cover the duplicate-with-conflicting-body_hash case.
The dedup vector uses identical
body_hashvalues for the duplicated(main, ob_place_order)key, so it proves dedup happens but not which entry survives when duplicates carry different bodies. A vector where the duplicate has a differingbody_hashwould pin down the canonical resolution rule.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extensions/tn_utils/maa_test.go` around lines 62 - 94, Test TestComputeRulesHash_OrderIndependentAndDedup currently verifies deduplication only when duplicated (namespace, action) entries share the same body_hash; add an additional case that inserts a duplicate (e.g., same "main" + "ob_place_order") with a different body_hash to assert which body wins the canonicalization. Update the test to call computeRulesHash with the conflicting-body duplicate (use repeatByte with a different value or nil vs non-nil), compute the expected canonical hash (by constructing the canonical single-entry input or by reordering inputs so the survivor is clear), and assert bytes.Equal(base, conflicted) or compare against the explicit expected hash to pin down the conflict-resolution rule for computeRulesHash.
161-170: 💤 Low valueConsider also asserting on a bad
unrestrictedlength.The test covers 19-byte
restrictedand 31-byterules_hash, but not a wrong-lengthunrestricted, leaving that validation branch unexercised.♻️ Optional addition
if _, err := deriveMAAAddress(good20, good20, repeatByte(0x33, 31), nil); err == nil { t.Fatal("expected error for 31-byte rules_hash") } + if _, err := deriveMAAAddress(good20, repeatByte(0x22, 21), good32, nil); err == nil { + t.Fatal("expected error for 21-byte unrestricted") + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extensions/tn_utils/maa_test.go` around lines 161 - 170, The test TestDeriveMAAAddress_RejectsBadLengths misses exercising the invalid-length branch for the unrestricted argument; add an assertion that calling deriveMAAAddress with a wrong-length unrestricted slice (e.g., 21 bytes produced by repeatByte) along with valid good20/good32 for restricted and rules_hash returns an error. Update the test to call deriveMAAAddress(repeatByte(0x11, 21), good20, good32, nil) and fail the test if err == nil so the unrestricted-length validation in deriveMAAAddress is covered.
172-183: ⚡ Quick winAdd a test for mismatched allow-list array lengths.
computeRulesHashreceivesnamespaces,actions, andbody_hashesas parallel slices. The validation tests never exercise the case where these lengths differ, which is the most likely path to an index-out-of-range panic in the implementation. A test asserting an error (rather than a panic) for unequal lengths would guard that contract.♻️ Suggested addition
if _, err := computeRulesHash("bps", 0, "0", "eth_truf", []string{"main"}, []string{"a"}, [][]byte{repeatByte(0x00, 31)}); err == nil { t.Fatal("expected error for 31-byte body_hash") } + // Mismatched parallel-array lengths must error, not panic. + if _, err := computeRulesHash("bps", 0, "0", "eth_truf", + []string{"main", "main"}, []string{"a"}, [][]byte{nil}); err == nil { + t.Fatal("expected error for mismatched allow-list array lengths") + } }Confirm how
computeRulesHashhandles unequal slice lengths today (error vs. panic) before adding the expectation:#!/bin/bash # Locate computeRulesHash and inspect its length validation / iteration over the parallel arrays. ast-grep --pattern 'func computeRulesHash($$$) ($_, $_) { $$$ }' rg -nP -C3 '\b(namespaces|actions|body_hashes)\b' --type=go -g '!*_test.go'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@extensions/tn_utils/maa_test.go` around lines 172 - 183, The test suite lacks a case where the parallel slices passed to computeRulesHash (namespaces, actions, body_hashes) have mismatched lengths; add a unit test in TestComputeRulesHash_Validation that calls computeRulesHash with differing lengths (e.g., namespaces length 1, actions length 2, body_hashes length 1) and assert it returns a non-nil error (or if computeRulesHash currently panics, wrap the call in a recover/assert that a panic occurs); reference computeRulesHash to locate validation logic and ensure the test covers the unequal-length path rather than causing an index-out-of-range panic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@extensions/tn_utils/maa.go`:
- Around line 227-232: The dedup logic collapses entries by (namespace, action)
and silently overwrites differing body_hash values; update the loop that builds
dedup (using maaAllowEntry, namespaces, actions, bodyHashes and the dedup map)
to detect conflicts: compute key := namespace+"\x00"+action, and if dedup
already has an entry with the same key but a different bodyHash, reject by
returning an error (or propagate an appropriate error) from the caller (e.g.,
the compute_rules_hash path) instead of overwriting; alternatively, if the spec
treats body_hash as part of identity, include bodyHash in the dedup key to keep
both entries—choose one behavior and implement the corresponding
check/early-return or key change.
---
Nitpick comments:
In `@extensions/tn_utils/maa_test.go`:
- Around line 62-94: Test TestComputeRulesHash_OrderIndependentAndDedup
currently verifies deduplication only when duplicated (namespace, action)
entries share the same body_hash; add an additional case that inserts a
duplicate (e.g., same "main" + "ob_place_order") with a different body_hash to
assert which body wins the canonicalization. Update the test to call
computeRulesHash with the conflicting-body duplicate (use repeatByte with a
different value or nil vs non-nil), compute the expected canonical hash (by
constructing the canonical single-entry input or by reordering inputs so the
survivor is clear), and assert bytes.Equal(base, conflicted) or compare against
the explicit expected hash to pin down the conflict-resolution rule for
computeRulesHash.
- Around line 161-170: The test TestDeriveMAAAddress_RejectsBadLengths misses
exercising the invalid-length branch for the unrestricted argument; add an
assertion that calling deriveMAAAddress with a wrong-length unrestricted slice
(e.g., 21 bytes produced by repeatByte) along with valid good20/good32 for
restricted and rules_hash returns an error. Update the test to call
deriveMAAAddress(repeatByte(0x11, 21), good20, good32, nil) and fail the test if
err == nil so the unrestricted-length validation in deriveMAAAddress is covered.
- Around line 172-183: The test suite lacks a case where the parallel slices
passed to computeRulesHash (namespaces, actions, body_hashes) have mismatched
lengths; add a unit test in TestComputeRulesHash_Validation that calls
computeRulesHash with differing lengths (e.g., namespaces length 1, actions
length 2, body_hashes length 1) and assert it returns a non-nil error (or if
computeRulesHash currently panics, wrap the call in a recover/assert that a
panic occurs); reference computeRulesHash to locate validation logic and ensure
the test covers the unequal-length path rather than causing an
index-out-of-range panic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d12bc0fe-b8f3-482d-8c21-64f581728fae
📒 Files selected for processing (3)
extensions/tn_utils/maa.goextensions/tn_utils/maa_test.goextensions/tn_utils/precompiles.go
✅ Actions performedComments resolved. Approval is disabled; enable |
resolves: https://github.com/truflation/website/issues/4035
What
Part 1 of the Modular Agent Addresses (MAA) rule store: the two pure
tn_utilsprecompiles that derive adeterministic agent-wallet address. Go only, no database, no consensus change.
Changes
compute_rules_hash(...)→keccak256of the canonical rule serialization (fee + allow-list, with a canonicaldedup/sort and a version byte).
derive_maa_address(...)→ low 20 bytes ofkeccak256(version ‖ restricted ‖ unrestricted ‖ rules_hash ‖ salt).maa_test.go).Testing
go vet ./extensions/tn_utils/— cleango test ./extensions/tn_utils/ -run 'MAA|RulesHash|DeriveMAA'— passNotes
Summary by CodeRabbit
derive_maa_addressandcompute_rules_hashprecompile functions to tn_utils for deterministic Modular Agent Address generation and rules hashing with automatic deduplication and canonical sorting.