Skip to content

fix: init v1 tree with custom forester#2319

Merged
ananas-block merged 3 commits intomainfrom
jorrit/fix-init-v1-tree-with-custom-forester
Mar 2, 2026
Merged

fix: init v1 tree with custom forester#2319
ananas-block merged 3 commits intomainfrom
jorrit/fix-init-v1-tree-with-custom-forester

Conversation

@ananas-block
Copy link
Contributor

@ananas-block ananas-block commented Mar 1, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Enforced network fee rules when no forester is registered to prevent invalid configurations and ensure consistent validation.
  • Tests
    • Updated test state setup to propagate and validate network fee values, improving parity between runtime behavior and tests.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 1, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 13c857d and da0fc6a.

⛔ Files ignored due to path filters (1)
  • program-tests/utils/src/e2e_test_env.rs is excluded by none and included by none
📒 Files selected for processing (3)
  • programs/registry/src/lib.rs
  • sdk-libs/program-test/src/indexer/state_tree.rs
  • sdk-libs/program-test/src/indexer/test_indexer.rs

📝 Walkthrough

Walkthrough

Adds a guard in registry checks that enforces rollover_metadata.network_fee == 0 when no forester_pda is registered but the authority claims forester access, and threads a new network_fee: u64 field through SDK test indexer StateMerkleTreeBundle and its construction paths.

Changes

Cohort / File(s) Summary
Registry: forester ↔ network_fee guards
programs/registry/src/lib.rs
Add a guard path in check_forester that when forester_pda is absent but authority matches forester access, enforces rollover_metadata.network_fee == 0, returning InvalidNetworkFee otherwise.
SDK test indexer: network_fee plumbing
sdk-libs/program-test/src/indexer/state_tree.rs, sdk-libs/program-test/src/indexer/test_indexer.rs
Add network_fee: u64 to StateMerkleTreeBundle; initialize and propagate network_fee in TestIndexer::new and add_state_merkle_tree (0 when forester present, default otherwise) into bundles pushed to state_merkle_trees.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

ai-review

Suggested reviewers

  • sergeytimoshin
  • SwenSchaeferjohann

Poem

🌲 A forester's absent, the ledger stands clear,
Fees set to zero where no keeper is near.
Bundles carry the numbers, consistent and neat,
Registry and tests now march in the beat. ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly describes the main change: initializing v1 tree with custom forester and enforcing network fee constraints based on forester presence.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 70.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch jorrit/fix-init-v1-tree-with-custom-forester

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

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@sdk-libs/program-test/src/indexer/test_indexer.rs`:
- Around line 1713-1723: The StateV2 branch is not accounting for a provided
forester while you later compute network_fee from forester before constructing
StateMerkleTreeBundle, which can create inconsistent metadata; update the
TreeType::StateV2 initialization (the match arm that constructs the StateV2
merkle tree) to either derive its network fee/config from forester the same way
as other tree types or add an explicit guard that rejects/returns an error when
forester.is_some() for TreeType::StateV2; ensure the change keeps
StateMerkleTreeBundle.network_fee in sync with how the tree was actually
initialized (referencing TreeType::StateV2, forester, network_fee, and
StateMerkleTreeBundle).

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2aefe89 and 13c857d.

⛔ Files ignored due to path filters (2)
  • external/photon is excluded by none and included by none
  • program-tests/utils/src/e2e_test_env.rs is excluded by none and included by none
📒 Files selected for processing (2)
  • sdk-libs/program-test/src/indexer/state_tree.rs
  • sdk-libs/program-test/src/indexer/test_indexer.rs

Copy link
Contributor

@SwenSchaeferjohann SwenSchaeferjohann left a comment

Choose a reason for hiding this comment

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

lgtm but those external/photon seem to be old?
does git submodule update external/photon fix it?

@ananas-block ananas-block force-pushed the jorrit/fix-init-v1-tree-with-custom-forester branch from 13c857d to a0183dc Compare March 2, 2026 20:00
@ananas-block
Copy link
Contributor Author

reverted the the photon submodule diff

@ananas-block ananas-block merged commit 845c4d3 into main Mar 2, 2026
31 checks passed
@ananas-block ananas-block deleted the jorrit/fix-init-v1-tree-with-custom-forester branch March 2, 2026 21:33
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.

2 participants