feat(reputation): receipt-gated reviews via Bubblegum V2 - #17
Open
blockiosaurus wants to merge 1 commit into
Open
feat(reputation): receipt-gated reviews via Bubblegum V2#17blockiosaurus wants to merge 1 commit into
blockiosaurus wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (18)
📒 Files selected for processing (21)
💤 Files with no reviewable changes (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (16)
WalkthroughReplaces the prior ChangesReviews Flow: IDL, Program, and JS Client
Sequence Diagram(s)sequenceDiagram
rect rgba(173, 216, 230, 0.5)
Note over Reviewer,Bubblegum: Setup (one-time)
Reviewer->>create_reviews_collection_v1: CreateReviewsCollectionV1 (payer)
create_reviews_collection_v1->>MplCore: CPI CreateCollectionV2 (BubblegumV2 + PermanentFreezeDelegate)
Reviewer->>register_reviews_tree_v1: RegisterReviewsTreeV1 (tree_index, depth params)
register_reviews_tree_v1->>SystemProgram: create_account (merkle tree, PDA seeds)
register_reviews_tree_v1->>Bubblegum: CPI CreateTreeConfigV2 (reviews authority PDA)
end
rect rgba(144, 238, 144, 0.5)
Note over Reviewer,Bubblegum: Leave Review
Reviewer->>leave_review_v1: LeaveReviewV1 (rating, feedbackUri, receipt proof)
leave_review_v1->>leave_review_v1: validate PDAs, programs, rating, feedbackUri length
leave_review_v1->>leave_review_v1: reconstruct Bubblegum leaf hash (agent+client creator binding)
leave_review_v1->>SPLCompression: verify_leaf CPI (root/leaf/index/proof nodes)
SPLCompression-->>leave_review_v1: Ok / Err (leaf mismatch)
leave_review_v1->>SystemProgram: create ReviewRecordV1 PDA (idempotency guard)
leave_review_v1->>Bubblegum: MintV2 CPI (review cNFT, reviews authority PDA signer)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
1 task
blockiosaurus
added a commit
that referenced
this pull request
Jun 22, 2026
The mintWorkReceipt suite previously only covered failure cases, relying on PR #17's leaveReview tests to incidentally exercise the success flow. Add a dedicated happy path that: - mints a receipt via the canonical PDAs - derives the resulting Bubblegum asset id - locally computes the leaf hash via hashReceiptLeaf (the helper was unused in this PR until now) - confirms the on-chain merkle root has moved off the empty-tree default Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
blockiosaurus
force-pushed
the
feat/reviews
branch
from
June 22, 2026 21:03
c9b82b5 to
71ff8bd
Compare
blockiosaurus
added a commit
that referenced
this pull request
Jun 23, 2026
#16) * feat(tools): work receipts via Bubblegum V2 + permissionless bootstrap Adds MintWorkReceiptV1, plus the receipts collection + tree machinery needed to host receipt cNFTs: - CreateReceiptsCollectionV1 — permissionless, idempotent bootstrap of the canonical receipts collection at ["receipts_collection"] with update_authority = ["receipts_authority"] PDA. - RegisterReceiptsTreeV1 — permissionless tree creation at PDA ["receipts_tree", index_le] with tree_creator = receipts_authority PDA. Caller picks an unused tree_index; first-come-first-served. - MintWorkReceiptV1 — authorized by an existing ExecutionDelegateRecordV1. Mints a soulbound work-receipt cNFT to the client wallet, with the agent asset and client both recorded as creators (agent: share 100, client: share 0 — for review-time binding without requiring a mint-time client signature). The receipts authority PDA does double duty as the collection's update_authority AND every tree's tree_creator, so the program signs all Bubblegum CPIs via invoke_signed with a single constant seed. No singleton config account; no captured admin. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(ci): make kinobi-tools post-gen step idempotent The standalone PDA helpers were written with 4-space indent but the committed copy had been reformatted to 2-space, so 'pnpm generate' on a clean checkout produced a diff and CI's 'working directory is clean' guard failed. Write the helpers pre-formatted to match the repo's prettier config (2-space, single quotes, trailing commas) so generation is byte-for- byte deterministic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(tools): CloseWorkReceiptV1 + PermanentBurnDelegate on collection Adds a spam-cleanup path for work receipts: clients who receive unsolicited receipts can burn them. - CreateReceiptsCollectionV1 now attaches PermanentBurnDelegate with authority = UpdateAuthority alongside the existing PermanentFreezeDelegate. The receipts_authority PDA is the collection's update authority, so the program is the burn signer. - New CloseWorkReceiptV1 instruction: the caller signs as the leaf owner; the program verifies the canonical PDAs (tree, collection, authority), then CPIs Bubblegum BurnV2 signed by the receipts_authority PDA. Args mirror Bubblegum's BurnV2InstructionArgs (root, data_hash, creator_hash, asset_data_hash, flags, nonce, index) plus tree_index for PDA derivation. Proof path passes through as remaining accounts. Closing is permitted regardless of whether the receipt has been reviewed — review cNFTs live in a separate tree and persist independently. This is the simpler model and matches "the owner retains full control of their wallet." Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(tools): switch CloseWorkReceiptV1Args to Pod zero-copy ABI CloseWorkReceiptV1Args has only fixed-size fields, so it should follow the repo's Pod/bytemuck convention used by RegisterExecutiveV1Args, RegisterReceiptsTreeV1Args, and CreateReceiptsCollectionV1Args. Borsh is reserved for args with variable-length fields (MintWorkReceiptV1Args and LeaveReviewV1Args carry a String URI). - repr(C) + #[derive(Pod, Zeroable)] on the struct - #[skip] discriminator + #[padding] alignment fields - Replace deserialize_close_work_receipt_args (Borsh) with cast_close_work_receipt_args (bytemuck::from_bytes) - Dispatcher now passes the full instruction_data slice to the cast helper, matching the other Pod handlers Total args size: 168 bytes, 8-byte aligned. Compile-time assertions guard both invariants. The JS client surface is unchanged — kinobi strips #[skip]/#[padding] fields so users still pass the same logical args. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(tools): add align_of assertion to CloseWorkReceiptV1Args size % 8 == 0 doesn't strictly prove align_of == 8 (a struct of [u8; 168] would also satisfy that). With #[repr(C)] + the u64 field the alignment is 8 in practice, but assert it explicitly so future field reorderings can't accidentally drop alignment without tripping a compile error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(tools): switch MintWorkReceiptV1Args to Pod + variable string tail Brings the last Borsh handler in mpl-agent-tools onto the repo's zero-copy ABI convention, using the Genesis program's pattern for variable-length string fields: - repr(C) + Pod/Zeroable on the args struct - discriminator(#[skip]) + padding(#[padding]) + tree_index(u64) live in the fixed-size head (16 bytes, 8-byte aligned) - receipt_uri is a zero-sized [u8; 0] sentinel tagged #[idl_type("String")]. The actual UTF-8 bytes are appended as a length-prefixed Borsh-style string tail past the Pod struct - Processor takes raw instruction_data, splits at size_of::<Args>(), bytemuck::from_bytes the head, parses the tail string with bounds + MAX_RECEIPT_URI_LEN + UTF-8 checks - Drop the now-unused borsh dep from tools/Cargo.toml Kinobi renders receiptUri as a string field via the idl_type annotation, so the generated JS/Rust client surface is unchanged — callers still pass receiptUri: '...' and the serializer emits the exact wire layout the program expects (head bytes + Borsh string tail). All 43 JS tests + Rust client tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(tools): also validate ExecutionDelegateRecord discriminator byte Owner + length checks alone don't prove the account is actually a delegate record — a different PDA owned by this program at the same size would slip through. Pin data[0] == Key::ExecutionDelegateRecordV1 as defense-in-depth. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: narrow bootstrap maybe() to only swallow already-initialized errors The old maybe() helper suppressed every failure from the idempotent bootstrap calls. That hides real setup regressions and lets failure- path tests pass for unrelated reasons. Now matches only the specific 'custom program error: 0x14' signature (MplAgentToolsError::ReceiptsCollectionAlreadyInitialized) in the error message / cause / program logs and rethrows anything else. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(tools): add happy-path mintWorkReceiptV1 test The mintWorkReceipt suite previously only covered failure cases, relying on PR #17's leaveReview tests to incidentally exercise the success flow. Add a dedicated happy path that: - mints a receipt via the canonical PDAs - derives the resulting Bubblegum asset id - locally computes the leaf hash via hashReceiptLeaf (the helper was unused in this PR until now) - confirms the on-chain merkle root has moved off the empty-tree default Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: re-enable default ava concurrency The 'concurrency: 1' guard was defensive against the old singleton- config + next_tree_index races. Both are gone: each test grabs a random tree index via randomU64() and the canonical collection PDA is idempotent (maybe() now narrowly swallows only the already-initialized error). Validated 4 consecutive full-suite runs — 44/44 each, ~6.8s wall- clock vs ~40s sequential. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
blockiosaurus
force-pushed
the
feat/reviews
branch
2 times, most recently
from
June 23, 2026 19:55
b1d9890 to
a045b26
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
programs/mpl-agent-reputation/src/state/seeds.rs (1)
50-81: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHoist duplicated receipts-prefix constants to module level.
RECEIPTS_COLLECTION_PREFIXis redeclared in bothcheck_receipts_collection_pdaandreceipts_collection_address, andRECEIPTS_TREE_PREFIXis local tocheck_receipts_tree_pda. Promote them to module-level constants (matching theREVIEWS_*prefixes) so the seed strings have a single source of truth.🤖 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 `@programs/mpl-agent-reputation/src/state/seeds.rs` around lines 50 - 81, The constants RECEIPTS_COLLECTION_PREFIX (used in both check_receipts_collection_pda and receipts_collection_address functions) and RECEIPTS_TREE_PREFIX (used in check_receipts_tree_pda function) are currently declared as local const within their respective functions, causing duplication. Move these two constants to module-level scope outside of all functions, following the same pattern as the existing REVIEWS_* prefix constants, then update each function to reference the module-level constants instead of declaring their own local const declarations.programs/mpl-agent-reputation/src/processor/leave_review.rs (1)
50-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
LeaveReviewV1Argsdeviates from the program's zero-copy instruction-arg pattern.
CreateReviewsCollectionV1Args/RegisterReviewsTreeV1Argsare fixed-size Pod structs cast via bytemuck, butLeaveReviewV1Argsis borsh-deserialized and carries aString, so it is neither#[repr(C)]norPod + Zeroable. This is defensible becausefeedback_uriis variable-length, but it does break the uniform zero-copy contract the rest of the program follows. Please confirm this deviation is intentional (vs. a fixed-size URI buffer).As per coding guidelines, "All account structs and instruction args must use
#[repr(C)], bePod + Zeroable, 8-byte aligned with compile-time size assertions, following zero-copy via bytemuck pattern".🤖 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 `@programs/mpl-agent-reputation/src/processor/leave_review.rs` around lines 50 - 80, The LeaveReviewV1Args struct deviates from the program's zero-copy pattern by using Borsh deserialization with a String field (feedback_uri) instead of being a fixed-size Pod struct with #[repr(C)] like CreateReviewsCollectionV1Args and RegisterReviewsTreeV1Args. Either convert feedback_uri from a String to a fixed-size byte array buffer (with appropriate compile-time size assertions and Pod + Zeroable trait implementations), or add clear documentation explaining why this structure intentionally breaks the uniform zero-copy contract. Ensure any fixed-size buffer approach aligns with the program's 8-byte alignment and zero-copy deserialization requirements.Source: Coding guidelines
programs/mpl-agent-reputation/src/processor/register_reviews_tree_v1.rs (1)
20-22: 📐 Maintainability & Code Quality | 🔵 TrivialCentralize
MPL_ACCOUNT_COMPRESSION_IDto avoid maintenance drift.This constant is duplicated across two processor modules:
register_reviews_tree_v1.rs(line 21, private) andleave_review.rs(line 30, public). Both define the same hardcoded pubkey value and use it for security-critical program guard checks. While currently synchronized, maintaining separate copies creates risk of silent divergence. Hoist into a shared module (e.g.,constants.rsorstate) and import in both processors to enforce consistency.🤖 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 `@programs/mpl-agent-reputation/src/processor/register_reviews_tree_v1.rs` around lines 20 - 22, The constant MPL_ACCOUNT_COMPRESSION_ID is duplicated in register_reviews_tree_v1.rs and leave_review.rs with identical hardcoded values, creating risk of maintenance drift. Create or use an existing constants module, define MPL_ACCOUNT_COMPRESSION_ID as a public constant there, remove the duplicate const definition from both register_reviews_tree_v1.rs and leave_review.rs, and add module imports in both processor files to reference the centralized constant instead.Source: Path instructions
programs/mpl-agent-reputation/src/error.rs (1)
27-79: 🩺 Stability & Availability | 🔵 TrivialEnsure the JS bootstrap error-code mapping stays in sync when reordering error variants.
The
ReviewsCollectionAlreadyInitializedvariant is currently at position 13, which maps to the on-chain Custom error code0xd. The generatedmplAgentReputation.tsis auto-synced viapnpm generate, but the manual hardcode atclients/js/test/_receiptsReviews.ts:83(REVIEWS_COLLECTION_ALREADY_INITIALIZED_HEX = '0xd') will break silently if error variants are reordered without runningpnpm generate. Consider adding a comment or doc reference in the error enum clarifying this cross-layer contract, or adding a lint/test to catch variant reordering.🤖 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 `@programs/mpl-agent-reputation/src/error.rs` around lines 27 - 79, The error variants in the Rust error enum are mapped to on-chain error codes by their position, and there are corresponding JS mappings (both auto-generated and manually hardcoded) that depend on this order. Add a clarifying doc comment or comment block near the top of the error enum (before or after the first variant) that explicitly documents this cross-layer contract between the Rust error variant order and the JS bootstrap error-code mappings, warning that reordering variants will break the mapping and requires running pnpm generate to resync both the auto-generated and manual hardcoded values. This will prevent silent failures when variants are accidentally reordered.
🤖 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 `@clients/js/test/reputation/leaveReview.test.ts`:
- Around line 116-122: The throwsAsync call in the leaveReviewV1 test lacks a
specific error matcher, which means it will pass for any thrown error rather
than specifically validating the duplicate-review failure. Add a matcher
argument to throwsAsync that explicitly checks for the expected duplicate-review
error type or error message. This ensures the test is actually proving the
idempotency gate behavior works correctly and isn't just passing when an
unrelated error occurs.
In `@clients/js/test/reputation/leaveReviewValidation.test.ts`:
- Around line 103-232: The test cases for leaveReviewV1 (rejects rating 0,
rejects rating 6, rejects empty feedback URI, rejects leafOwner mismatch, and
all other validation tests through rejects bogus receipt data_hash) are only
asserting that an error was thrown, not validating that the specific intended
validation check failed. Capture the error returned from each t.throwsAsync call
and add assertions on the error message or code to verify the rejection is from
the expected validation path. Follow the pattern already established in the
tests on lines 252-269 where error details are inspected to ensure each test
validates the correct validation failure.
In `@programs/mpl-agent-reputation/Cargo.toml`:
- Line 25: The mpl-agent-tools dependency in Cargo.toml includes a full client
library with generated account types that use std::io::Error, making it
non-SBF-compliant, but the program only needs the program ID constant for PDA
derivations. Replace the dependency on the full mpl-agent-tools client crate
with a lightweight alternative that provides just the program ID constant,
either by extracting the ID into a separate lightweight shared constant crate or
by defining the ID constant directly in this program's crate to eliminate the
unnecessary std dependency and reduce bloat.
In `@programs/mpl-agent-reputation/src/processor/register_reviews_tree_v1.rs`:
- Around line 125-152: The merkle_tree_account_size function is duplicated
identically between register_reviews_tree_v1.rs and register_receipts_tree_v1.rs
(with only comment differences), and the MPL_ACCOUNT_COMPRESSION_ID constant is
duplicated privately across multiple processor files despite being exported only
from leave_review.rs with no other imports. Extract both
merkle_tree_account_size and MPL_ACCOUNT_COMPRESSION_ID to a shared utility
module (such as in programs/mpl-agent-shared or a common submodule), make
MPL_ACCOUNT_COMPRESSION_ID public, and update all processor files including
register_reviews_tree_v1.rs and register_receipts_tree_v1.rs to import and use
these definitions from the single shared source. Additionally, validate the
HEADER_SIZE constant (88 bytes) across the full range of max_depth,
max_buffer_size, and canopy_depth parameter combinations used by clients, or
document the empirical derivation clearly if a library-provided constant is
unavailable.
---
Nitpick comments:
In `@programs/mpl-agent-reputation/src/error.rs`:
- Around line 27-79: The error variants in the Rust error enum are mapped to
on-chain error codes by their position, and there are corresponding JS mappings
(both auto-generated and manually hardcoded) that depend on this order. Add a
clarifying doc comment or comment block near the top of the error enum (before
or after the first variant) that explicitly documents this cross-layer contract
between the Rust error variant order and the JS bootstrap error-code mappings,
warning that reordering variants will break the mapping and requires running
pnpm generate to resync both the auto-generated and manual hardcoded values.
This will prevent silent failures when variants are accidentally reordered.
In `@programs/mpl-agent-reputation/src/processor/leave_review.rs`:
- Around line 50-80: The LeaveReviewV1Args struct deviates from the program's
zero-copy pattern by using Borsh deserialization with a String field
(feedback_uri) instead of being a fixed-size Pod struct with #[repr(C)] like
CreateReviewsCollectionV1Args and RegisterReviewsTreeV1Args. Either convert
feedback_uri from a String to a fixed-size byte array buffer (with appropriate
compile-time size assertions and Pod + Zeroable trait implementations), or add
clear documentation explaining why this structure intentionally breaks the
uniform zero-copy contract. Ensure any fixed-size buffer approach aligns with
the program's 8-byte alignment and zero-copy deserialization requirements.
In `@programs/mpl-agent-reputation/src/processor/register_reviews_tree_v1.rs`:
- Around line 20-22: The constant MPL_ACCOUNT_COMPRESSION_ID is duplicated in
register_reviews_tree_v1.rs and leave_review.rs with identical hardcoded values,
creating risk of maintenance drift. Create or use an existing constants module,
define MPL_ACCOUNT_COMPRESSION_ID as a public constant there, remove the
duplicate const definition from both register_reviews_tree_v1.rs and
leave_review.rs, and add module imports in both processor files to reference the
centralized constant instead.
In `@programs/mpl-agent-reputation/src/state/seeds.rs`:
- Around line 50-81: The constants RECEIPTS_COLLECTION_PREFIX (used in both
check_receipts_collection_pda and receipts_collection_address functions) and
RECEIPTS_TREE_PREFIX (used in check_receipts_tree_pda function) are currently
declared as local const within their respective functions, causing duplication.
Move these two constants to module-level scope outside of all functions,
following the same pattern as the existing REVIEWS_* prefix constants, then
update each function to reference the module-level constants instead of
declaring their own local const declarations.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5703d7c9-bae7-43f3-bf71-66e641df6030
⛔ Files ignored due to path filters (18)
Cargo.lockis excluded by!**/*.lockclients/js/src/generated/reputation/accounts/index.tsis excluded by!**/generated/**clients/js/src/generated/reputation/accounts/reviewRecordV1.tsis excluded by!**/generated/**clients/js/src/generated/reputation/accounts/standalonePdas.tsis excluded by!**/generated/**clients/js/src/generated/reputation/errors/mplAgentReputation.tsis excluded by!**/generated/**clients/js/src/generated/reputation/instructions/createReviewsCollectionV1.tsis excluded by!**/generated/**clients/js/src/generated/reputation/instructions/index.tsis excluded by!**/generated/**clients/js/src/generated/reputation/instructions/leaveReviewV1.tsis excluded by!**/generated/**clients/js/src/generated/reputation/instructions/registerReviewsTreeV1.tsis excluded by!**/generated/**clients/js/src/generated/reputation/types/key.tsis excluded by!**/generated/**clients/rust-reputation/src/generated/accounts/mod.rsis excluded by!**/generated/**clients/rust-reputation/src/generated/accounts/review_record_v1.rsis excluded by!**/generated/**clients/rust-reputation/src/generated/errors/mpl_agent_reputation.rsis excluded by!**/generated/**clients/rust-reputation/src/generated/instructions/create_reviews_collection_v1.rsis excluded by!**/generated/**clients/rust-reputation/src/generated/instructions/leave_review_v1.rsis excluded by!**/generated/**clients/rust-reputation/src/generated/instructions/mod.rsis excluded by!**/generated/**clients/rust-reputation/src/generated/instructions/register_reviews_tree_v1.rsis excluded by!**/generated/**clients/rust-reputation/src/generated/types/key.rsis excluded by!**/generated/**
📒 Files selected for processing (21)
clients/js/src/index.tsclients/js/src/reputation/index.tsclients/js/test/_receiptsReviews.tsclients/js/test/reputation/leaveReview.test.tsclients/js/test/reputation/leaveReviewValidation.test.tsclients/js/test/reputation/register.test.tsclients/rust-reputation/tests/create.rsconfigs/kinobi-reputation.cjsidls/mpl_agent_reputation.jsonprograms/mpl-agent-reputation/Cargo.tomlprograms/mpl-agent-reputation/src/error.rsprograms/mpl-agent-reputation/src/instruction.rsprograms/mpl-agent-reputation/src/processor/create_reviews_collection_v1.rsprograms/mpl-agent-reputation/src/processor/leave_review.rsprograms/mpl-agent-reputation/src/processor/mod.rsprograms/mpl-agent-reputation/src/processor/register.rsprograms/mpl-agent-reputation/src/processor/register_reviews_tree_v1.rsprograms/mpl-agent-reputation/src/state/agent_reputation.rsprograms/mpl-agent-reputation/src/state/mod.rsprograms/mpl-agent-reputation/src/state/review_record.rsprograms/mpl-agent-reputation/src/state/seeds.rs
💤 Files with no reviewable changes (4)
- clients/js/test/reputation/register.test.ts
- clients/rust-reputation/tests/create.rs
- programs/mpl-agent-reputation/src/state/agent_reputation.rs
- programs/mpl-agent-reputation/src/processor/register.rs
Comment on lines
+125
to
+152
| /// Compute the merkle tree account size in bytes, matching | ||
| /// spl-concurrent-merkle-tree's struct layout. `Path<MAX_DEPTH>` has no | ||
| /// leaf field — verified empirically against Bubblegum's canopy check. | ||
| fn merkle_tree_account_size( | ||
| max_depth: usize, | ||
| max_buffer_size: usize, | ||
| canopy_depth: usize, | ||
| ) -> Result<usize, ProgramError> { | ||
| const HEADER_SIZE: usize = 88; | ||
| const NODE_SIZE: usize = 32; | ||
| let change_log_size = NODE_SIZE + max_depth * NODE_SIZE + 4 + 4; | ||
| let change_logs_total = change_log_size | ||
| .checked_mul(max_buffer_size) | ||
| .ok_or(ProgramError::ArithmeticOverflow)?; | ||
| let scalars = 24; | ||
| let path_size = max_depth * NODE_SIZE + 4 + 4; | ||
| let canopy_size = if canopy_depth == 0 { | ||
| 0 | ||
| } else { | ||
| let nodes = (1usize << (canopy_depth + 1)) | ||
| .checked_sub(2) | ||
| .ok_or(ProgramError::ArithmeticOverflow)?; | ||
| nodes | ||
| .checked_mul(NODE_SIZE) | ||
| .ok_or(ProgramError::ArithmeticOverflow)? | ||
| }; | ||
| Ok(HEADER_SIZE + scalars + change_logs_total + path_size + canopy_size) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for an existing size helper/header constant in the compression dependency.
rg -nP 'CONCURRENT_MERKLE_TREE_HEADER_SIZE|merkle_tree_get_size|fn .*account_size|struct Path' \
$(fd -t d -i 'compression' ~/.cargo 2>/dev/null || true) . -g '!target' || true
# Confirm which depth/buffer combos clients pass.
rg -nP 'TREE_MAX_DEPTH|TREE_MAX_BUFFER|maxDepth|maxBufferSize|canopyDepth' clients/js -C1Repository: metaplex-foundation/mpl-agent
Length of output: 5675
🏁 Script executed:
#!/bin/bash
# Check both merkle_tree_account_size functions for duplication
echo "=== register_reviews_tree_v1.rs ==="
sed -n '128,152p' programs/mpl-agent-reputation/src/processor/register_reviews_tree_v1.rs
echo -e "\n=== register_receipts_tree_v1.rs ==="
sed -n '137,161p' programs/mpl-agent-tools/src/processor/register_receipts_tree_v1.rsRepository: metaplex-foundation/mpl-agent
Length of output: 2131
🏁 Script executed:
#!/bin/bash
# Search for MPL_ACCOUNT_COMPRESSION_ID across all processors
echo "=== MPL_ACCOUNT_COMPRESSION_ID definitions ==="
rg -n 'MPL_ACCOUNT_COMPRESSION_ID' programs/ --type rsRepository: metaplex-foundation/mpl-agent
Length of output: 241
🏁 Script executed:
#!/bin/bash
# Check Cargo.toml for mpl-account-compression dependency version
fd -n 'Cargo.toml' programs/mpl-agent-reputation programs/mpl-agent-tools -x grep -l 'mpl-account-compression\|spl.*merkle'
rg -A2 'mpl-account-compression|spl.*merkle' $(fd -n 'Cargo.toml' programs/) || echo "Not found in Cargo.toml"Repository: metaplex-foundation/mpl-agent
Length of output: 653
🏁 Script executed:
#!/bin/bash
# Look for the canopy calculation and verify the shift operation constraint
echo "=== Verifying canopy shift operation ==="
rg -n '1usize << ' programs/mpl-agent-reputation/src/processor/register_reviews_tree_v1.rs -B2 -A2Repository: metaplex-foundation/mpl-agent
Length of output: 385
🏁 Script executed:
#!/bin/bash
# Search for MPL_ACCOUNT_COMPRESSION_ID (fixing the flag issue)
echo "=== MPL_ACCOUNT_COMPRESSION_ID definitions ==="
rg 'MPL_ACCOUNT_COMPRESSION_ID' programs/ -A2 -B2Repository: metaplex-foundation/mpl-agent
Length of output: 7193
🏁 Script executed:
#!/bin/bash
# Check Cargo.toml for dependencies
echo "=== reputation Cargo.toml ==="
cat programs/mpl-agent-reputation/Cargo.toml | grep -A5 'dependencies'
echo -e "\n=== tools Cargo.toml ==="
cat programs/mpl-agent-tools/Cargo.toml | grep -A5 'dependencies'Repository: metaplex-foundation/mpl-agent
Length of output: 553
🏁 Script executed:
#!/bin/bash
# Look for any mpl-account-compression or spl-concurrent-merkle-tree references
echo "=== Searching for compression/merkle dependency references ==="
rg 'mpl.account.compression|spl.*merkle|compression' programs/ Cargo.toml --max-depth 2 -iRepository: metaplex-foundation/mpl-agent
Length of output: 230
🏁 Script executed:
#!/bin/bash
# Check if leave_review.rs uses MPL_ACCOUNT_COMPRESSION_ID (mentioned in review)
echo "=== Looking for leave_review.rs and MPL_ACCOUNT_COMPRESSION_ID usage ==="
find programs/ -name 'leave_review.rs' -o -name '*leave*review*' | head -5
rg 'leave_review|MPL_ACCOUNT_COMPRESSION' programs/mpl-agent-reputation -lRepository: metaplex-foundation/mpl-agent
Length of output: 483
🏁 Script executed:
#!/bin/bash
# Check if there's a module-level constant or shared location for these constants
echo "=== Checking mod.rs files for constant exports ==="
find programs/mpl-agent-reputation/src -name 'mod.rs' -exec cat {} \;
echo -e "\n=== Checking tools mod.rs ==="
find programs/mpl-agent-tools/src -name 'mod.rs' -exec cat {} \;Repository: metaplex-foundation/mpl-agent
Length of output: 7371
🏁 Script executed:
#!/bin/bash
# Verify the exact line-by-line comparison of both merkle_tree_account_size implementations
echo "=== Full comparison of both functions ==="
echo "REPUTATION (lines 128-152):"
sed -n '128,152p' programs/mpl-agent-reputation/src/processor/register_reviews_tree_v1.rs
echo -e "\nTOOLS (lines 137-161):"
sed -n '137,161p' programs/mpl-agent-tools/src/processor/register_receipts_tree_v1.rs | head -25Repository: metaplex-foundation/mpl-agent
Length of output: 2151
🏁 Script executed:
#!/bin/bash
# Check if leave_review.rs is in the same file or different, and how it relates
echo "=== leave_review.rs location and structure ==="
ls -la programs/mpl-agent-reputation/src/processor/leave_review.rs
wc -l programs/mpl-agent-reputation/src/processor/leave_review.rsRepository: metaplex-foundation/mpl-agent
Length of output: 388
🏁 Script executed:
#!/bin/bash
# Check if leave_review.rs exports MPL_ACCOUNT_COMPRESSION_ID for use elsewhere
echo "=== leave_review.rs pub const declaration ==="
grep -n "pub const MPL_ACCOUNT_COMPRESSION_ID" programs/mpl-agent-reputation/src/processor/leave_review.rs
# Check if register_reviews_tree_v1.rs actually uses the one from leave_review
echo -e "\n=== register_reviews_tree_v1.rs imports/uses ==="
head -30 programs/mpl-agent-reputation/src/processor/register_reviews_tree_v1.rs | grep -E '^use|^const|^pub'Repository: metaplex-foundation/mpl-agent
Length of output: 676
🏁 Script executed:
#!/bin/bash
# Look at spl-concurrent-merkle-tree documentation/source to validate the constants
echo "=== Searching for spl-concurrent-merkle-tree references in codebase ==="
rg -r 'spl.concurrent.*merkle|concurrent.*merkle' . --max-depth 3
# Check if there's any Cargo.lock or workspace that shows the indirect dependency
echo -e "\n=== Workspace Cargo.toml ==="
find . -maxdepth 2 -name 'Cargo.toml' -exec grep -l 'workspace\|members' {} \;
cat Cargo.toml 2>/dev/null || echo "No root Cargo.toml"Repository: metaplex-foundation/mpl-agent
Length of output: 10657
🏁 Script executed:
#!/bin/bash
# Get the full picture of MPL_ACCOUNT_COMPRESSION_ID definitions and see if they're all the same value
echo "=== All MPL_ACCOUNT_COMPRESSION_ID definitions with values ==="
rg 'const MPL_ACCOUNT_COMPRESSION_ID.*=' programs/ -A1 -B1
# Check if there's a shared constant module or if they should be consolidated
echo -e "\n=== Check if leave_review's pub const is actually used elsewhere ==="
rg 'use.*leave_review' programs/ -A5
rg 'leave_review::MPL_ACCOUNT_COMPRESSION_ID' programs/ || echo "Not imported from leave_review"Repository: metaplex-foundation/mpl-agent
Length of output: 2830
🌐 Web query:
spl-concurrent-merkle-tree header size constant account layout HEADER_SIZE 88
💡 Result:
The SPL Concurrent Merkle Tree “header size constant” you’re asking about (HEADER_SIZE = 88) could not be found in the authoritative sources I located (Solana Program Library repos and docs.rs). What I did find: - The on-chain ConcurrentMerkleTree account struct layout is defined as #[repr(C)] with fields sequence_number (u64), active_index (u64), buffer_size (u64), change_logs ([ChangeLog<MAX_DEPTH>; MAX_BUFFER_SIZE]), and rightmost_proof (Path<MAX_DEPTH]) [1]. - The on-chain header data types used by account-compression SDKs include fields like maxBufferSize, maxDepth, authority, creationSlot, isBatchInitialized, and an 8-byte alignment padding region [2]. - The SPL account-compression SDK defines valid (maxDepth, maxBufferSize) tuples, but this doesn’t mention a HEADER_SIZE constant [3]. If you can share where you saw “HEADER_SIZE 88” (file path, crate name, or a link/snippet), I can verify it precisely against the correct source and explain how it’s computed (or correct it if it’s version-specific).
Citations:
- 1: https://github.com/solana-labs/solana-program-library/blob/master/libraries/concurrent-merkle-tree/src/concurrent_merkle_tree.rs
- 2: https://cdn.jsdelivr.net/npm/@metaplex-foundation/mpl-account-compression@0.0.1/dist/src/generated/types/concurrentMerkleTreeHeaderData.d.ts
- 3: https://github.com/solana-labs/solana-program-library/blob/master/account-compression/sdk/src/constants/index.ts
Extract duplicated merkle_tree_account_size function and MPL_ACCOUNT_COMPRESSION_ID constant to a shared module.
The function at lines 128–152 is byte-for-byte identical to programs/mpl-agent-tools/src/processor/register_receipts_tree_v1.rs:137–161 (only toolbox version has comments). Similarly, MPL_ACCOUNT_COMPRESSION_ID is defined privately in register_reviews_tree_v1.rs and four other processor files across both programs; only leave_review.rs exports it as public, yet no other module imports it. This violates the coding guideline: "duplicated logic between programs must be consistent."
Extract both to a shared utility module (e.g., programs/mpl-agent-shared/src/lib.rs or a common submodule in each program's library) and re-export or use from a single source of truth.
Additionally, the hand-rolled merkle tree size calculation (HEADER_SIZE=88 bytes) is reverse-engineered against the spl-concurrent-merkle-tree layout and tested only with (max_depth=5, max_buffer_size=8, canopy_depth=0). Validate this constant across the full range of parameter combinations supported by clients, or document the empirical derivation clearly if a library-provided constant is unavailable.
🤖 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 `@programs/mpl-agent-reputation/src/processor/register_reviews_tree_v1.rs`
around lines 125 - 152, The merkle_tree_account_size function is duplicated
identically between register_reviews_tree_v1.rs and register_receipts_tree_v1.rs
(with only comment differences), and the MPL_ACCOUNT_COMPRESSION_ID constant is
duplicated privately across multiple processor files despite being exported only
from leave_review.rs with no other imports. Extract both
merkle_tree_account_size and MPL_ACCOUNT_COMPRESSION_ID to a shared utility
module (such as in programs/mpl-agent-shared or a common submodule), make
MPL_ACCOUNT_COMPRESSION_ID public, and update all processor files including
register_reviews_tree_v1.rs and register_receipts_tree_v1.rs to import and use
these definitions from the single shared source. Additionally, validate the
HEADER_SIZE constant (88 bytes) across the full range of max_depth,
max_buffer_size, and canopy_depth parameter combinations used by clients, or
document the empirical derivation clearly if a library-provided constant is
unavailable.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
programs/mpl-agent-reputation/src/processor/leave_review.rs (1)
353-357: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStale version reference in the doc comment.
The comment states the SDK's newer versions conflict with "our pinned solana-program 2.3", but this crate now builds against solana-program 3.0.0. Update the note so the rationale for the hand-rolled
verify_leafCPI isn't tied to an outdated version pin.🤖 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 `@programs/mpl-agent-reputation/src/processor/leave_review.rs` around lines 353 - 357, The doc comment above the verify_leaf instruction construction references an outdated solana-program version (2.3), but the crate now builds against solana-program 3.0.0. Update the doc comment that explains why the hand-rolled verify_leaf CPI implementation is necessary to reflect the current solana-program version and ensure the rationale for not using the SDK crate is still accurate and relevant for version 3.0.0.
🤖 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.
Nitpick comments:
In `@programs/mpl-agent-reputation/src/processor/leave_review.rs`:
- Around line 353-357: The doc comment above the verify_leaf instruction
construction references an outdated solana-program version (2.3), but the crate
now builds against solana-program 3.0.0. Update the doc comment that explains
why the hand-rolled verify_leaf CPI implementation is necessary to reflect the
current solana-program version and ensure the rationale for not using the SDK
crate is still accurate and relevant for version 3.0.0.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 73980f92-85c4-4b41-b4bd-1ed165abbc7f
⛔ Files ignored due to path filters (18)
Cargo.lockis excluded by!**/*.lockclients/js/src/generated/reputation/accounts/index.tsis excluded by!**/generated/**clients/js/src/generated/reputation/accounts/reviewRecordV1.tsis excluded by!**/generated/**clients/js/src/generated/reputation/accounts/standalonePdas.tsis excluded by!**/generated/**clients/js/src/generated/reputation/errors/mplAgentReputation.tsis excluded by!**/generated/**clients/js/src/generated/reputation/instructions/createReviewsCollectionV1.tsis excluded by!**/generated/**clients/js/src/generated/reputation/instructions/index.tsis excluded by!**/generated/**clients/js/src/generated/reputation/instructions/leaveReviewV1.tsis excluded by!**/generated/**clients/js/src/generated/reputation/instructions/registerReviewsTreeV1.tsis excluded by!**/generated/**clients/js/src/generated/reputation/types/key.tsis excluded by!**/generated/**clients/rust-reputation/src/generated/accounts/mod.rsis excluded by!**/generated/**clients/rust-reputation/src/generated/accounts/review_record_v1.rsis excluded by!**/generated/**clients/rust-reputation/src/generated/errors/mpl_agent_reputation.rsis excluded by!**/generated/**clients/rust-reputation/src/generated/instructions/create_reviews_collection_v1.rsis excluded by!**/generated/**clients/rust-reputation/src/generated/instructions/leave_review_v1.rsis excluded by!**/generated/**clients/rust-reputation/src/generated/instructions/mod.rsis excluded by!**/generated/**clients/rust-reputation/src/generated/instructions/register_reviews_tree_v1.rsis excluded by!**/generated/**clients/rust-reputation/src/generated/types/key.rsis excluded by!**/generated/**
📒 Files selected for processing (21)
clients/js/src/index.tsclients/js/src/reputation/index.tsclients/js/test/_receiptsReviews.tsclients/js/test/reputation/leaveReview.test.tsclients/js/test/reputation/leaveReviewValidation.test.tsclients/js/test/reputation/register.test.tsclients/rust-reputation/tests/create.rsconfigs/kinobi-reputation.cjsidls/mpl_agent_reputation.jsonprograms/mpl-agent-reputation/Cargo.tomlprograms/mpl-agent-reputation/src/error.rsprograms/mpl-agent-reputation/src/instruction.rsprograms/mpl-agent-reputation/src/processor/create_reviews_collection_v1.rsprograms/mpl-agent-reputation/src/processor/leave_review.rsprograms/mpl-agent-reputation/src/processor/mod.rsprograms/mpl-agent-reputation/src/processor/register.rsprograms/mpl-agent-reputation/src/processor/register_reviews_tree_v1.rsprograms/mpl-agent-reputation/src/state/agent_reputation.rsprograms/mpl-agent-reputation/src/state/mod.rsprograms/mpl-agent-reputation/src/state/review_record.rsprograms/mpl-agent-reputation/src/state/seeds.rs
💤 Files with no reviewable changes (4)
- clients/rust-reputation/tests/create.rs
- programs/mpl-agent-reputation/src/state/agent_reputation.rs
- clients/js/test/reputation/register.test.ts
- programs/mpl-agent-reputation/src/processor/register.rs
✅ Files skipped from review due to trivial changes (2)
- clients/js/src/index.ts
- clients/js/src/reputation/index.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- clients/js/test/reputation/leaveReview.test.ts
- programs/mpl-agent-reputation/src/state/review_record.rs
- programs/mpl-agent-reputation/src/processor/register_reviews_tree_v1.rs
- programs/mpl-agent-reputation/src/state/mod.rs
- clients/js/test/_receiptsReviews.ts
- configs/kinobi-reputation.cjs
- programs/mpl-agent-reputation/src/processor/create_reviews_collection_v1.rs
- programs/mpl-agent-reputation/src/instruction.rs
- programs/mpl-agent-reputation/src/error.rs
- programs/mpl-agent-reputation/src/state/seeds.rs
- clients/js/test/reputation/leaveReviewValidation.test.ts
- idls/mpl_agent_reputation.json
Adds LeaveReviewV1, plus the reviews collection + tree machinery needed to host review cNFTs. Builds on the work-receipt primitive shipped in the previous PR. - CreateReviewsCollectionV1 — permissionless, idempotent bootstrap of the canonical reviews collection at ["reviews_collection"] with update_authority = ["reviews_authority"] PDA. Soulbound: every review cNFT inherits a PermanentFreezeDelegate. - RegisterReviewsTreeV1 — permissionless tree creation at PDA ["reviews_tree", index_le] with tree_creator = reviews_authority PDA. Caller picks an unused tree_index. - LeaveReviewV1 — reviewer must hold the work-receipt cNFT being reviewed; supplies the Bubblegum leaf proof + on-chain reconstruction binds the receipt to both the reviewed agent AND the reviewer (creator_hash includes both addresses). A ReviewRecordV1 PDA seeded by the receipt's asset id is the idempotency gate against double-review. Cross-program canonicalization: the receipts collection account passed to LeaveReviewV1 is verified to be the PDA derived from mpl-agent-tools's program id, eliminating spoof vectors. The reputation program no longer ships an AgentReputationV1 PDA or RegisterReputationV1 instruction — review aggregates are computable off-chain from the review cNFTs and ReviewRecordV1 PDAs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
blockiosaurus
force-pushed
the
feat/reviews
branch
from
June 23, 2026 21:51
a045b26 to
7bfd15c
Compare
danenbm
approved these changes
Jul 4, 2026
danenbm
left a comment
danenbm
left a comment
Contributor
There was a problem hiding this comment.
Looks great! Approved with some minor ideas and one potential minor gotcha.
Comment on lines
+66
to
+70
| if ctx.accounts.merkle_tree.data_len() != 0 | ||
| || *ctx.accounts.merkle_tree.owner != system_program::id() | ||
| { | ||
| return Err(MplAgentReputationError::InvalidAccountData.into()); | ||
| } |
Contributor
There was a problem hiding this comment.
Nit: could be more descriptive "tree already exists" error.
| let receipts_collection = ctx.accounts.receipts_collection.key; | ||
| let receipt_asset_id = get_asset_id(receipts_merkle_tree, args.receipt_nonce); | ||
| let receipt_owner = *ctx.accounts.reviewer.key; | ||
| let receipt_delegate = receipt_owner; |
Contributor
There was a problem hiding this comment.
If the receipt owner delegates their receipt, this assumption will not hold.
Comment on lines
+144
to
+148
| // The leaf owner of the review cNFT must be the agent's wallet. | ||
| if *ctx.accounts.leaf_owner.key != asset_owner { | ||
| return Err(MplAgentReputationError::LeafOwnerMismatch.into()); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
After this wdyt about this random design idea?:
if *ctx.accounts.reviewer.key == *ctx.accounts.leaf_owner.key {
return Err(MplAgentReputationError::SelfReviewNotAllowed.into());
}
with:
/// 17 - Cannot review your own agent
#[error("Cannot leave a review for your own agent")]
SelfReviewNotAllowed,
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the reviews side of the system, built on top of the work-receipt primitive shipped in #16:
CreateReviewsCollectionV1— permissionless, idempotent bootstrap of the canonical reviews collection at `["reviews_collection"]` with `update_authority = ["reviews_authority"]` PDA. Soulbound: every review cNFT inherits a `PermanentFreezeDelegate`.RegisterReviewsTreeV1— permissionless tree creation at PDA `["reviews_tree", index_le]` with `tree_creator = reviews_authority` PDA. Caller picks an unused `tree_index`.LeaveReviewV1— reviewer must hold the work-receipt cNFT being reviewed; supplies the Bubblegum leaf proof, and on-chain reconstruction binds the receipt to both the reviewed agent AND the reviewer (creator_hash includes both addresses). A `ReviewRecordV1` PDA seeded by the receipt's asset id is the idempotency gate against double-review.Cross-program canonicalization: the receipts collection account passed to `LeaveReviewV1` is verified to be the PDA derived from `mpl-agent-tools`'s program id, eliminating spoof vectors.
Note: the reputation program no longer ships an `AgentReputationV1` PDA or `RegisterReputationV1` instruction — review aggregates are computable off-chain from the review cNFTs and `ReviewRecordV1` PDAs.
This is PR 2 of 3 stacked PRs. Stacked on #16 (work receipts). PR 3 adds discovery docs.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes