Open
Conversation
Deduplicate the level-15 proof node shared by both leaves (saves 32B) and shrink the discriminator from 4 bytes to 1 byte (saves 3B). Total instruction data drops from 1042B to 1007B, fitting the v0 transaction within the 1232-byte limit with 4 bytes margin.
…uplication Add nullify_dedup instruction that packs 2-4 nullifications into a single transaction using proof node deduplication. Nearby Merkle tree leaves share sibling nodes at common ancestor levels; the encoding stores each unique node once and uses bitvecs/2-bit source fields to reconstruct all proofs on-chain. - 1-byte custom discriminator [79], reuses NullifyLeaves accounts struct - Encoding: shared_top_node (level 15) + bitvec for proof_2 + 2-bit source fields for proof_3/proof_4, with u32::MAX sentinels for count < 4 - MAX_NODES=28 verified by tx size test (1230 bytes with ALT + compute budget ix) - SDK: compress_proofs() encoder, create_nullify_dedup_instruction() builder, nullify_dedup_lookup_table_accounts() helper - Unit tests: data size, accounts, discriminator collision, round-trip, edge cases - Integration tests: 4-leaf, 3-leaf, 2-leaf success + 1-leaf rejection
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Address Lookup Table (ALT) support and V1 multi-nullify batching: new CLI/config flags, ALT propagation through epoch manager and batch contexts, PreparedTransaction/v0 support, proof deduplication and multi-nullify instructions in the registry program, and test/Justfile updates for ALT-driven flows. Changes
Sequence Diagram(s)sequenceDiagram
participant Forester as Forester Service
participant EpochMgr as Epoch Manager
participant TxBuilder as V1 Tx Builder
participant Helper as V1 Helpers
participant SolanaRPC as Solana RPC
participant Registry as Registry Program
Forester->>EpochMgr: run_service() (load LUT -> Arc<Vec<ALT>>)
EpochMgr->>TxBuilder: build_signed_transaction_batch(..., address_lookup_tables, enable_v1_multi_nullify)
TxBuilder->>TxBuilder: decide use_multi_nullify (flag && ALT present && queue thresholds)
TxBuilder->>Helper: fetch_proofs_and_create_instructions(use_multi_nullify)
Helper->>Helper: group_state_items_for_dedup() & compress_proofs()
Helper-->>TxBuilder: return labeled instructions (multi or single)
TxBuilder->>Forester: return PreparedTransaction (versioned or legacy)
Forester->>SolanaRPC: send PreparedTransaction
SolanaRPC->>Registry: execute nullify_state_v1_multi (or single)
Registry->>Registry: reconstruct proofs -> process_nullify_state_v1_multi()
Registry-->>SolanaRPC: success
SolanaRPC-->>Forester: confirmation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
…ioned transactions, and tx size fix - Add min_queue_items config (CLI --min-queue-items, default 5000) to delay V1 state nullification processing until enough items accumulate for optimal dedup grouping - Integrate nullify_dedup into forester: group_state_items_for_dedup greedy algorithm forms groups of 4, 3, 2 with shared proof compression (70% savings observed) - Support versioned transactions with address lookup tables for dedup instructions - Reduce NULLIFY_DEDUP_MAX_NODES from 28 to 27 to fit within 1232-byte tx limit when both SetComputeUnitLimit and SetComputeUnitPrice are included - Add CompressedProofs struct replacing tuple return from compress_proofs - Remove nullify_2 instruction (superseded by nullify_dedup) - Add slot advancement in e2e test for surfpool offline mode - Add dedup grouping log assertion in e2e test
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
forester/tests/legacy/test_utils.rs (1)
97-108:⚠️ Potential issue | 🔴 Critical
GeneralConfiginitialization is incomplete and will not compile.The
GeneralConfigstruct requires three additional fields to be initialized:
queue_polling_mode(required)group_authority(optional, but explicitly declared)helius_rpc(required)These fields were added to the struct definition and must be provided here unless defaults are available. Without them, this test utility won't compile.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@forester/tests/legacy/test_utils.rs` around lines 97 - 108, GeneralConfig initialization is missing required fields; update the GeneralConfig literal to include queue_polling_mode (set to a valid QueuePollingMode enum variant used in your codebase), group_authority (explicitly provide either None or a valid GroupAuthority value as declared in the struct), and helius_rpc (provide a valid HeliusRpc config/URL or the default HeliusRpcConfig value used elsewhere); keep the other fields as-is so the test compiles against the new struct definition.forester/src/processor/v1/tx_builder.rs (1)
135-148:⚠️ Potential issue | 🟠 MajorDon't treat every
"not found"as an indexer miss.This branch now skips the batch for any error string containing
"not found". That will also swallow unrelated failures like missing lookup tables or missing on-chain accounts, and because the hashes were already inserted intoprocessed_hash_cache, a real fault can now be delayed instead of surfaced.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@forester/src/processor/v1/tx_builder.rs` around lines 135 - 148, The current Err(e) branch in tx_builder.rs treats any error whose string contains "not found" as an indexer miss and returns Ok, which swallows unrelated failures and can hide real faults after processed_hash_cache was updated; change this to only treat true indexer-miss errors by pattern-matching or downcasting the error to the concrete indexer/DB "record not found" error type (e.g., sqlx::Error::RowNotFound or your indexer client error enum) instead of substring matching, and if you cannot downcast reliably add an explicit helper like is_indexer_record_not_found(&e) that checks the concrete error variant; also ensure processed_hash_cache is only updated (or entries removed) after confirming success/intentional skip so real errors are propagated (referencing the Err(e) branch, processed_hash_cache, instructions and last_valid_block_height).forester/src/epoch_manager.rs (1)
3257-3278:⚠️ Potential issue | 🟠 MajorReused v2 processors can keep stale ALT snapshots across epoch transitions.
Processors are reused, but their
BatchContextgets ALT data from a one-time cloned snapshot. After ALT changes, reused processors won’t see new lookup-table contents unless recreated or explicitly refreshed.💡 Suggested fix (safe fallback)
- if let Some(entry) = self.state_processors.get(&tree_accounts.merkle_tree) { + if let Some(entry) = self.state_processors.get(&tree_accounts.merkle_tree) { let (stored_epoch, processor_ref) = entry.value(); let processor_clone = processor_ref.clone(); let old_epoch = *stored_epoch; drop(entry); // Release read lock before any async operation - if old_epoch != epoch_info.epoch { - // Update epoch in the map (processor is reused with its cached state) - debug!( - "Reusing StateBatchProcessor for tree {} across epoch transition ({} -> {})", - tree_accounts.merkle_tree, old_epoch, epoch_info.epoch - ); - self.state_processors.insert( - tree_accounts.merkle_tree, - (epoch_info.epoch, processor_clone.clone()), - ); - // Update the processor's epoch context and phases - processor_clone - .lock() - .await - .update_epoch(epoch_info.epoch, epoch_info.phases.clone()); - } - return Ok(processor_clone); + if old_epoch == epoch_info.epoch { + return Ok(processor_clone); + } + + // Recreate on epoch transition to pick up latest ALT snapshot. + self.state_processors.remove(&tree_accounts.merkle_tree); }Also applies to: 3283-3291, 3321-3340, 3345-3353
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@forester/src/epoch_manager.rs` around lines 3257 - 3278, The reused StateBatchProcessor can hold a stale ALT snapshot across epoch transitions; instead of only calling processor_clone.lock().await.update_epoch(...), either refresh the processor's BatchContext ALT snapshot or replace the processor with a freshly initialized instance so it picks up current lookup-table data. Concretely, in the old_epoch != epoch_info.epoch branch that manipulates state_processors and calls update_epoch, (a) add and call a method like refresh_alt_snapshot() on the locked processor (or) (b) construct a new StateBatchProcessor for tree_accounts.merkle_tree with the current epoch/phases and swap it into state_processors (updating the map key value to (epoch_info.epoch, new_processor.clone())); ensure the new method or constructor re-clones current ALT snapshot into BatchContext so subsequent processing sees updated lookup-table contents.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@forester/src/epoch_manager.rs`:
- Around line 3073-3076: The code unconditionally applies the ALT-based
min_queue_items gating (using alt_snapshot, address_lookup_tables, and
batched_tx_config.min_queue_items = self.config.min_queue_items) to all V1
trees; change it so the assignment only occurs when the tree being processed is
TreeType::StateV1 (i.e., gate the assignment behind a check like if
batched_tx_config.tree_type == TreeType::StateV1 or the local variable that
represents the current tree type) so AddressV1 processing is not stalled by the
nullify-dedup queue threshold.
In `@forester/src/processor/v1/helpers.rs`:
- Around line 381-486: group_state_items_for_dedup currently groups items by
compressibility only, but create_nullify_dedup_instruction uses a single
change_log_index (derived from root_seq % STATE_MERKLE_TREE_CHANGELOG) for the
whole group; fix by ensuring groups are split further by identical
change_log_index before building instructions: after grouping (groups from
group_state_items_for_dedup) compute each item's change_log_index =
items_with_proofs[idx].1.root_seq % STATE_MERKLE_TREE_CHANGELOG and partition
any group that contains mixed change_log_index values into subgroups with
identical change_log_index values (preserving order), then proceed to
compress_proofs and call create_nullify_dedup_instruction per subgroup; apply
the same split logic to the other dedup handling block that mirrors this code
path.
In `@forester/src/processor/v1/send_transaction.rs`:
- Around line 393-395: The code currently forces a default (all-zero) signature
by calling prepared_transaction.signature().unwrap_or_default(); change
tx_signature to be an Option<Signature> (i.e., let tx_signature =
prepared_transaction.signature();) and remove the unwrap_or_default; then update
any downstream logging/reporting that uses tx_signature to handle the Option
(log only when Some(sig) and avoid writing a synthetic default), referencing
prepared_transaction.signature() and the tx_signature variable when making the
changes.
In `@forester/src/processor/v1/tx_builder.rs`:
- Around line 152-165: The loop builds versioned transactions with up to
config.batch_size instructions but NULLIFY_DEDUP_MAX_NODES was calculated only
for a single dedup + compute-budget instruction set, so v0 messages can exceed
the 1232-byte limit when batch_size > 1; update the logic around
create_smart_transaction (and the instruction_chunk handling) to either force
single-instruction batches when a dedup/ALT-related instruction is present
(detect by opcode/name of the dedup instruction) or, after calling
create_smart_transaction, serialize/measure the v0 message size and, if it
exceeds the limit, split the instruction_chunk and retry (effectively falling
back to per-instruction batching). Ensure you reference NULLIFY_DEDUP_MAX_NODES
when deciding the guard and keep address_lookup_tables handling unchanged while
splitting/retrying.
In `@forester/tests/e2e_test.rs`:
- Around line 643-666: The test currently only prints warnings when the "logs"
directory or any "forester.*" file is missing, which allows the dedup assertion
to be skipped; change the behavior in the is_v1_state_test_enabled() branch so
missing logs or missing forester files cause the test to fail: replace the
println! warnings with assert!(false, "...") or use panic! with a clear message
indicating the logs/ directory or forester.* log file is required for this test,
and keep the existing logic that reads latest_log, sets content and checks
has_dedup (variables/functions to locate: is_v1_state_test_enabled, log_dir,
latest_log, content, has_dedup) so that absence of logs or absence of matching
files fails the test rather than merely warning.
In `@programs/registry/src/account_compression_cpi/nullify.rs`:
- Around line 97-124: Recompute and validate the dedup proof count inside
process_nullify_dedup using count_from_leaf_indices(leaf_indices) and reject any
leaf_indices layouts with non-trailing u32::MAX sentinels (e.g., [a,b,MAX,c]) by
returning Err(RegistryError::InvalidProofEncoding); then ensure the recomputed
count matches the caller-supplied count (or ignore the caller count and use the
recomputed value) before proceeding so garbage in trailing slots or mismatched
count vs. leaf_indices cannot be accepted.
In `@programs/registry/src/account_compression_cpi/sdk.rs`:
- Around line 123-132: The instruction uses a custom 1-byte discriminator for
NullifyDedup; remove that custom discriminator in the instruction definition
(the NullifyDedup instruction in lib.rs) so Anchor will derive the standard
8-byte discriminator from the instruction name, then rebuild so the sdk code
that constructs crate::instruction::NullifyDedup (in
programs/registry/src/account_compression_cpi/sdk.rs) emits the standard 8-byte
Anchor discriminator instead of the custom [79] byte. Ensure no other manual
discriminator attributes remain on NullifyDedup and recompile to pick up the
derived discriminator.
---
Outside diff comments:
In `@forester/src/epoch_manager.rs`:
- Around line 3257-3278: The reused StateBatchProcessor can hold a stale ALT
snapshot across epoch transitions; instead of only calling
processor_clone.lock().await.update_epoch(...), either refresh the processor's
BatchContext ALT snapshot or replace the processor with a freshly initialized
instance so it picks up current lookup-table data. Concretely, in the old_epoch
!= epoch_info.epoch branch that manipulates state_processors and calls
update_epoch, (a) add and call a method like refresh_alt_snapshot() on the
locked processor (or) (b) construct a new StateBatchProcessor for
tree_accounts.merkle_tree with the current epoch/phases and swap it into
state_processors (updating the map key value to (epoch_info.epoch,
new_processor.clone())); ensure the new method or constructor re-clones current
ALT snapshot into BatchContext so subsequent processing sees updated
lookup-table contents.
In `@forester/src/processor/v1/tx_builder.rs`:
- Around line 135-148: The current Err(e) branch in tx_builder.rs treats any
error whose string contains "not found" as an indexer miss and returns Ok, which
swallows unrelated failures and can hide real faults after processed_hash_cache
was updated; change this to only treat true indexer-miss errors by
pattern-matching or downcasting the error to the concrete indexer/DB "record not
found" error type (e.g., sqlx::Error::RowNotFound or your indexer client error
enum) instead of substring matching, and if you cannot downcast reliably add an
explicit helper like is_indexer_record_not_found(&e) that checks the concrete
error variant; also ensure processed_hash_cache is only updated (or entries
removed) after confirming success/intentional skip so real errors are propagated
(referencing the Err(e) branch, processed_hash_cache, instructions and
last_valid_block_height).
In `@forester/tests/legacy/test_utils.rs`:
- Around line 97-108: GeneralConfig initialization is missing required fields;
update the GeneralConfig literal to include queue_polling_mode (set to a valid
QueuePollingMode enum variant used in your codebase), group_authority
(explicitly provide either None or a valid GroupAuthority value as declared in
the struct), and helius_rpc (provide a valid HeliusRpc config/URL or the default
HeliusRpcConfig value used elsewhere); keep the other fields as-is so the test
compiles against the new struct definition.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 9bc4c478-5308-4429-aab6-fc24af9f7f8e
⛔ Files ignored due to path filters (2)
program-tests/registry-test/tests/nullify_dedup.rsis excluded by none and included by nonesdk-tests/csdk-anchor-full-derived-test/tests/integration_tests.rsis excluded by none and included by none
📒 Files selected for processing (18)
forester/justfileforester/src/cli.rsforester/src/config.rsforester/src/epoch_manager.rsforester/src/processor/v1/config.rsforester/src/processor/v1/helpers.rsforester/src/processor/v1/send_transaction.rsforester/src/processor/v1/tx_builder.rsforester/src/smart_transaction.rsforester/tests/e2e_test.rsforester/tests/legacy/test_utils.rsforester/tests/priority_fee_test.rsforester/tests/test_nullify_dedup_tx_size.rsforester/tests/test_utils.rsprograms/registry/src/account_compression_cpi/nullify.rsprograms/registry/src/account_compression_cpi/sdk.rsprograms/registry/src/errors.rsprograms/registry/src/lib.rs
Comment on lines
+393
to
+395
| let tx_signature = prepared_transaction | ||
| .signature() | ||
| .unwrap_or_default(); |
Contributor
There was a problem hiding this comment.
Do not synthesize a default signature when one is unavailable.
Line 395 turns “no signature yet” into an all-zero signature, which pollutes logs and failure correlation. Keep this as Option<Signature> and only log/report when present.
💡 Suggested fix
- let tx_signature = prepared_transaction
- .signature()
- .unwrap_or_default();
- let tx_signature_str = tx_signature.to_string();
+ let tx_signature = prepared_transaction.signature();
+ let tx_signature_str = tx_signature
+ .map(|s| s.to_string())
+ .unwrap_or_else(|| "<unavailable>".to_string());
@@
- TransactionSendResult::ExecutionFailure(error, Some(tx_signature))
+ TransactionSendResult::ExecutionFailure(error, tx_signature)
} else if is_confirmation_unknown {
TransactionSendResult::ConfirmationUnknown(
error,
- Some(tx_signature),
+ tx_signature,
)
} else {
- TransactionSendResult::SendFailure(error, Some(tx_signature))
+ TransactionSendResult::SendFailure(error, tx_signature)
}
@@
- TransactionSendResult::SendFailure(ForesterError::from(e), Some(tx_signature))
+ TransactionSendResult::SendFailure(ForesterError::from(e), tx_signature)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@forester/src/processor/v1/send_transaction.rs` around lines 393 - 395, The
code currently forces a default (all-zero) signature by calling
prepared_transaction.signature().unwrap_or_default(); change tx_signature to be
an Option<Signature> (i.e., let tx_signature =
prepared_transaction.signature();) and remove the unwrap_or_default; then update
any downstream logging/reporting that uses tx_signature to handle the Option
(log only when Some(sig) and avoid writing a synthetic default), referencing
prepared_transaction.signature() and the tx_signature variable when making the
changes.
Adds queue_item_count to BuildTransactionBatchConfig and disables multi-nullify when the queue is too large, falling back to single nullify for more reliable throughput. Renames use_dedup to use_multi_nullify for consistency.
time 0.3.46+ pulls time-core 0.1.8 which uses edition2024, unsupported by the Cargo 1.84 bundled with Solana platform-tools.
Harden leaf_indices validation to reject malformed layouts like [a, b, MAX, c] where sentinels appear in non-trailing positions.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
forester/tests/legacy/test_utils.rs (1)
97-108: 🧹 Nitpick | 🔵 TrivialSync GeneralConfig initialization between test utilities.
The non-legacy
forester/tests/test_utils.rsinitializesGeneralConfigwith three additional fields that the legacy version omits:queue_polling_mode,group_authority, andhelius_rpc. WhileGeneralConfigprovides defaults for these fields, the two test utility files should be kept synchronized to avoid subtle inconsistencies and make the test setup expectations explicit.Field differences
Non-legacy includes:
queue_polling_mode: QueuePollingMode::OnChain, group_authority: None, helius_rpc: false,Legacy omits these three fields.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@forester/tests/legacy/test_utils.rs` around lines 97 - 108, The GeneralConfig initialization in the legacy test utilities is missing three fields present in the non-legacy version; update the GeneralConfig struct literal used in forester/tests/legacy/test_utils.rs to include queue_polling_mode: QueuePollingMode::OnChain, group_authority: None, and helius_rpc: false so the test setup matches the non-legacy test_utils; locate the GeneralConfig instantiation (the struct literal containing slot_update_interval_seconds, tree_discovery_interval_seconds, etc.) and add those three fields explicitly to keep both utilities synchronized.
♻️ Duplicate comments (2)
programs/registry/src/account_compression_cpi/nullify.rs (1)
97-110: 🧹 Nitpick | 🔵 TrivialNon-trailing sentinel values are still accepted.
The current implementation would accept
[a, b, u32::MAX, c]and return count=2, silently ignoring the garbage value in slot 3. While this doesn't cause a security issue (the CPI paths are gated bycount), it's a validation gap that could mask caller bugs.The pattern-matching fix from the previous review would reject these malformed inputs explicitly:
Suggested stricter validation
pub fn count_from_leaf_indices(leaf_indices: &[u32; 4]) -> Result<usize> { - if leaf_indices[0] == u32::MAX || leaf_indices[1] == u32::MAX { - return err!(RegistryError::InvalidProofEncoding); - } - Ok(if leaf_indices[2] == u32::MAX { - 2 - } else if leaf_indices[3] == u32::MAX { - 3 - } else { - 4 - }) + match *leaf_indices { + [a, b, u32::MAX, u32::MAX] if a != u32::MAX && b != u32::MAX => Ok(2), + [a, b, c, u32::MAX] if a != u32::MAX && b != u32::MAX && c != u32::MAX => Ok(3), + [a, b, c, d] if a != u32::MAX && b != u32::MAX && c != u32::MAX && d != u32::MAX => Ok(4), + _ => err!(RegistryError::InvalidProofEncoding), + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@programs/registry/src/account_compression_cpi/nullify.rs` around lines 97 - 110, The function count_from_leaf_indices must reject non-trailing sentinel values; update its validation so after asserting leaf_indices[0] and [1] are not u32::MAX, explicitly handle patterns: if leaf_indices[2] == u32::MAX then require leaf_indices[3] == u32::MAX and return 2, else if leaf_indices[3] == u32::MAX return 3, else return 4; if a sentinel appears followed by a non-sentinel (e.g. [a,b,MAX,c]) return Err(RegistryError::InvalidProofEncoding). Ensure you modify count_from_leaf_indices to perform these explicit checks using leaf_indices and RegistryError::InvalidProofEncoding.forester/src/epoch_manager.rs (1)
3012-3015:⚠️ Potential issue | 🟠 MajorScope
min_queue_itemsgating toTreeType::StateV1only.The condition at line 3013 enables
min_queue_itemsfor any V1 tree when ALT is configured andenable_v1_multi_nullifyis true. However, multi-nullify deduplication is designed specifically for state trees. Applying this threshold toAddressV1trees could stall address tree processing behind queue thresholds intended for nullify dedup.The fix should add a tree type check:
🛡️ Proposed fix
let alt_snapshot = (*self.address_lookup_tables).clone(); -if self.config.enable_v1_multi_nullify && !alt_snapshot.is_empty() { +if self.config.enable_v1_multi_nullify + && !alt_snapshot.is_empty() + && tree_accounts.tree_type == TreeType::StateV1 +{ batched_tx_config.min_queue_items = self.config.min_queue_items; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@forester/src/epoch_manager.rs` around lines 3012 - 3015, The current gating sets batched_tx_config.min_queue_items whenever self.config.enable_v1_multi_nullify is true and alt_snapshot is non-empty, which wrongly applies the threshold to non-state V1 trees; update the condition to also check the current tree type and only set batched_tx_config.min_queue_items when the tree is TreeType::StateV1 (i.e., add an additional check on the tree type where this code lives, referencing self.config.enable_v1_multi_nullify, alt_snapshot, and batched_tx_config.min_queue_items so AddressV1 trees are not affected).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@forester/src/cli.rs`:
- Around line 290-296: The CLI flag min_queue_items is declared as Option<usize>
but has default_value = "5000", which forces Some(5000) and prevents None;
either change the intent or the CLI: if the intended runtime default is 5000,
make the type non-optional (usize) or update config docs in config.rs to state
the CLI always supplies 5000; if None must be reachable from the CLI, remove
default_value = "5000" from the min_queue_items arg so the parser can produce
None and update any code that assumes a value to handle the None case
accordingly (adjust places that read min_queue_items to treat None as immediate
processing).
In `@forester/src/config.rs`:
- Around line 33-38: Add validation in new_for_start to error when
enable_v1_multi_nullify is true but lookup_table_address is None: check
args.enable_v1_multi_nullify && args.lookup_table_address.is_none() and return a
ConfigError::InvalidArguments describing that "enable_v1_multi_nullify requires
lookup_table_address to be set" (mirroring the existing enable_priority_fees /
priority_fee_microlamports check style) so the config fails fast instead of
silently falling back to legacy behavior.
In `@forester/src/processor/v1/tx_builder.rs`:
- Around line 126-129: Extract the duplicated numeric threshold into a single
public constant and import it where needed: create a shared pub const
MULTI_NULLIFY_MAX_QUEUE_SIZE: usize = 10_000 in a common module (e.g.,
config.rs), then replace the local const MULTI_NULLIFY_MAX_QUEUE_SIZE in
tx_builder.rs and the duplicate in send_transaction.rs with uses of that shared
constant; update references in the conditional that uses
enable_v1_multi_nullify, address_lookup_tables.is_empty(), and
config.queue_item_count to use the imported constant.
---
Outside diff comments:
In `@forester/tests/legacy/test_utils.rs`:
- Around line 97-108: The GeneralConfig initialization in the legacy test
utilities is missing three fields present in the non-legacy version; update the
GeneralConfig struct literal used in forester/tests/legacy/test_utils.rs to
include queue_polling_mode: QueuePollingMode::OnChain, group_authority: None,
and helius_rpc: false so the test setup matches the non-legacy test_utils;
locate the GeneralConfig instantiation (the struct literal containing
slot_update_interval_seconds, tree_discovery_interval_seconds, etc.) and add
those three fields explicitly to keep both utilities synchronized.
---
Duplicate comments:
In `@forester/src/epoch_manager.rs`:
- Around line 3012-3015: The current gating sets
batched_tx_config.min_queue_items whenever self.config.enable_v1_multi_nullify
is true and alt_snapshot is non-empty, which wrongly applies the threshold to
non-state V1 trees; update the condition to also check the current tree type and
only set batched_tx_config.min_queue_items when the tree is TreeType::StateV1
(i.e., add an additional check on the tree type where this code lives,
referencing self.config.enable_v1_multi_nullify, alt_snapshot, and
batched_tx_config.min_queue_items so AddressV1 trees are not affected).
In `@programs/registry/src/account_compression_cpi/nullify.rs`:
- Around line 97-110: The function count_from_leaf_indices must reject
non-trailing sentinel values; update its validation so after asserting
leaf_indices[0] and [1] are not u32::MAX, explicitly handle patterns: if
leaf_indices[2] == u32::MAX then require leaf_indices[3] == u32::MAX and return
2, else if leaf_indices[3] == u32::MAX return 3, else return 4; if a sentinel
appears followed by a non-sentinel (e.g. [a,b,MAX,c]) return
Err(RegistryError::InvalidProofEncoding). Ensure you modify
count_from_leaf_indices to perform these explicit checks using leaf_indices and
RegistryError::InvalidProofEncoding.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 3fa5067b-55f1-4145-9ccc-e7e494199940
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockand included by noneCargo.tomlis excluded by none and included by noneprogram-tests/registry-test/tests/nullify_state_v1_multi.rsis excluded by none and included by none
📒 Files selected for processing (16)
forester/src/cli.rsforester/src/config.rsforester/src/epoch_manager.rsforester/src/processor/v1/config.rsforester/src/processor/v1/helpers.rsforester/src/processor/v1/send_transaction.rsforester/src/processor/v1/tx_builder.rsforester/tests/e2e_test.rsforester/tests/legacy/priority_fee_test.rsforester/tests/legacy/test_utils.rsforester/tests/priority_fee_test.rsforester/tests/test_nullify_state_v1_multi_tx_size.rsforester/tests/test_utils.rsprograms/registry/src/account_compression_cpi/nullify.rsprograms/registry/src/account_compression_cpi/sdk.rsprograms/registry/src/lib.rs
Replace the complex multi-scheme encoding (1-bit bitvec for proof_2, 2-bit source selectors for proof_3/proof_4, separate shared_top_node) with a uniform pool-based approach: - Deduplicated node pool built level-by-level across all proofs - Each proof (including proof_1) selects 16 nodes from the pool via a u32 bitvec using the bitvec crate - Removes proof_2_shared, proof_3_source, proof_4_source, shared_top_node - Adds proof_bitvecs: [u32; 4] - Bumps NULLIFY_STATE_V1_MULTI_MAX_NODES from 26 to 27 (10-byte margin) - Hardens count_from_leaf_indices to reject non-trailing sentinels
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
forester/src/processor/v1/helpers.rs (1)
427-478:⚠️ Potential issue | 🟠 MajorSplit multi-nullify groups on every instruction-wide key.
group_state_items_for_dedup()only groups by leaf order and compressibility, but the emitted multi instruction takes a singlemerkle_tree,nullifier_queue, andchange_log_indexfrom the first element. A compressible group that spans different trees/queues or differentroot_seq % STATE_MERKLE_TREE_CHANGELOGwill encode the later leaves against the wrong metadata and fail verification. Please partition on those fields before callingcompress_proofs(and add a mixed-tree / mixed-slot regression while fixing it).Also applies to: 532-557
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLAUDE.md`:
- Around line 180-183: The documentation header claiming "from forester/
directory" doesn't match the commands which use "-f forester/justfile"
(repo-root style); either remove the "-f forester/justfile" flags from the two
commands so they run from the forester/ directory (e.g., "just local" and "just
test") or change the header to say "from repository root" to match the existing
"just -f forester/justfile local" and "just -f forester/justfile test"
invocations—update the text and commands around the Forester just examples
accordingly.
- Around line 184-186: Add a second example invocation for running Forester e2e
tests in devnet mode next to the existing local-mode example; locate the
existing line "TEST_MODE=local cargo test --package forester e2e_test --
--nocapture" and add a sibling example "TEST_MODE=devnet cargo test --package
forester e2e_test -- --nocapture" (preserving the same flags and context) so
both TEST_MODE=local and TEST_MODE=devnet are documented.
In `@forester/tests/test_nullify_state_v1_multi_tx_size.rs`:
- Around line 73-82: Replace the hardcoded calculation for ix_data_size with the
actual serialized instruction length: generate/serialize the instruction bytes
used in the test (the same value passed into the transaction) and set
ix_data_size = serialized_instruction.bytes().len() (or equivalent) so the test
derives size from the real encoding instead of the manual formula; update
references around ix_data_size and NULLIFY_STATE_V1_MULTI_MAX_NODES in
test_nullify_state_v1_multi_tx_size.rs to use the serialized instruction length
for the printed assertion.
In `@programs/registry/src/account_compression_cpi/sdk.rs`:
- Around line 101-135: Add a development-time check inside
create_nullify_state_v1_multi_instruction to ensure inputs.nodes.len() does not
exceed NULLIFY_STATE_V1_MULTI_MAX_NODES by inserting a debug_assert that
compares inputs.nodes.len() <= NULLIFY_STATE_V1_MULTI_MAX_NODES and includes a
descriptive message like "nodes exceeds max: {} > {}", referencing
inputs.nodes.len() and NULLIFY_STATE_V1_MULTI_MAX_NODES; this will catch
oversized node arrays early during debugging without affecting release builds.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 50aa77d3-5647-4237-a5fa-9ed37ab77c45
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockand included by noneprogram-tests/registry-test/tests/nullify_state_v1_multi.rsis excluded by none and included by none
📒 Files selected for processing (7)
CLAUDE.mdforester/src/processor/v1/helpers.rsforester/tests/test_nullify_state_v1_multi_tx_size.rsprograms/registry/Cargo.tomlprograms/registry/src/account_compression_cpi/nullify.rsprograms/registry/src/account_compression_cpi/sdk.rsprograms/registry/src/lib.rs
Comment on lines
+180
to
+183
| # Using just (preferred, from forester/ directory): | ||
| just -f forester/justfile local # Run e2e test without rebuilding SBF programs | ||
| just -f forester/justfile test # Build SBF test deps first, then run e2e test | ||
|
|
Contributor
There was a problem hiding this comment.
Fix the working-directory mismatch in the Forester just commands.
Line 180 says “from forester/ directory”, but Line 181 and Line 182 use -f forester/justfile, which is a repository-root invocation pattern.
🛠️ Suggested doc fix
-# Using just (preferred, from forester/ directory):
-just -f forester/justfile local # Run e2e test without rebuilding SBF programs
-just -f forester/justfile test # Build SBF test deps first, then run e2e test
+# Using just (preferred, from repository root):
+just -f forester/justfile local # Run e2e test without rebuilding SBF programs
+just -f forester/justfile test # Build SBF test deps first, then run e2e test
+
+# If already in forester/ directory:
+just local
+just test📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Using just (preferred, from forester/ directory): | |
| just -f forester/justfile local # Run e2e test without rebuilding SBF programs | |
| just -f forester/justfile test # Build SBF test deps first, then run e2e test | |
| # Using just (preferred, from repository root): | |
| just -f forester/justfile local # Run e2e test without rebuilding SBF programs | |
| just -f forester/justfile test # Build SBF test deps first, then run e2e test | |
| # If already in forester/ directory: | |
| just local | |
| just test |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLAUDE.md` around lines 180 - 183, The documentation header claiming "from
forester/ directory" doesn't match the commands which use "-f forester/justfile"
(repo-root style); either remove the "-f forester/justfile" flags from the two
commands so they run from the forester/ directory (e.g., "just local" and "just
test") or change the header to say "from repository root" to match the existing
"just -f forester/justfile local" and "just -f forester/justfile test"
invocations—update the text and commands around the Forester just examples
accordingly.
Comment on lines
+184
to
186
| # Or directly: | ||
| TEST_MODE=local cargo test --package forester e2e_test -- --nocapture | ||
| ``` |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add a TEST_MODE=devnet Forester e2e example next to local mode.
This section currently documents only local mode; adding devnet keeps the test entrypoints complete for both supported environments.
🧪 Suggested addition
# Or directly:
TEST_MODE=local cargo test --package forester e2e_test -- --nocapture
+TEST_MODE=devnet cargo test --package forester e2e_test -- --nocaptureBased on learnings: "Run E2E tests with TEST_MODE=local cargo test for local validator testing or TEST_MODE=devnet for devnet testing".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Or directly: | |
| TEST_MODE=local cargo test --package forester e2e_test -- --nocapture | |
| ``` | |
| # Or directly: | |
| TEST_MODE=local cargo test --package forester e2e_test -- --nocapture | |
| TEST_MODE=devnet cargo test --package forester e2e_test -- --nocapture |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLAUDE.md` around lines 184 - 186, Add a second example invocation for
running Forester e2e tests in devnet mode next to the existing local-mode
example; locate the existing line "TEST_MODE=local cargo test --package forester
e2e_test -- --nocapture" and add a sibling example "TEST_MODE=devnet cargo test
--package forester e2e_test -- --nocapture" (preserving the same flags and
context) so both TEST_MODE=local and TEST_MODE=devnet are documented.
Comment on lines
+73
to
+82
| let ix_data_size = 8 + 2 + 8 + 16 + 16 + 4 + NULLIFY_STATE_V1_MULTI_MAX_NODES * 32; | ||
|
|
||
| println!( | ||
| "nullify_state_v1_multi v0 transaction size: {} bytes (limit: 1232)", | ||
| tx_size | ||
| ); | ||
| println!( | ||
| " nullify_state_v1_multi instruction data: {} bytes", | ||
| ix_data_size | ||
| ); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Derive ix_data_size from instruction bytes instead of a manual formula.
The hardcoded size expression can silently drift when instruction encoding changes.
♻️ Suggested refactor
- let ix_data_size = 8 + 2 + 8 + 16 + 16 + 4 + NULLIFY_STATE_V1_MULTI_MAX_NODES * 32;
+ let ix_data_size = nullify_ix.data.len();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@forester/tests/test_nullify_state_v1_multi_tx_size.rs` around lines 73 - 82,
Replace the hardcoded calculation for ix_data_size with the actual serialized
instruction length: generate/serialize the instruction bytes used in the test
(the same value passed into the transaction) and set ix_data_size =
serialized_instruction.bytes().len() (or equivalent) so the test derives size
from the real encoding instead of the manual formula; update references around
ix_data_size and NULLIFY_STATE_V1_MULTI_MAX_NODES in
test_nullify_state_v1_multi_tx_size.rs to use the serialized instruction length
for the printed assertion.
Comment on lines
+101
to
+135
| pub fn create_nullify_state_v1_multi_instruction( | ||
| inputs: CreateNullifyStateV1MultiInstructionInputs, | ||
| epoch: u64, | ||
| ) -> Instruction { | ||
| let register_program_pda = get_registered_program_pda(&crate::ID); | ||
| let registered_forester_pda = if inputs.is_metadata_forester { | ||
| None | ||
| } else { | ||
| Some(get_forester_epoch_pda_from_authority(&inputs.derivation, epoch).0) | ||
| }; | ||
| let (cpi_authority, _bump) = get_cpi_authority_pda(); | ||
| let instruction_data = crate::instruction::NullifyStateV1Multi { | ||
| change_log_index: inputs.change_log_index, | ||
| queue_indices: inputs.queue_indices, | ||
| leaf_indices: inputs.leaf_indices, | ||
| proof_bitvecs: inputs.proof_bitvecs, | ||
| nodes: inputs.nodes, | ||
| }; | ||
|
|
||
| let accounts = crate::accounts::NullifyLeaves { | ||
| authority: inputs.authority, | ||
| registered_forester_pda, | ||
| registered_program_pda: register_program_pda, | ||
| nullifier_queue: inputs.nullifier_queue, | ||
| merkle_tree: inputs.merkle_tree, | ||
| log_wrapper: NOOP_PUBKEY.into(), | ||
| cpi_authority, | ||
| account_compression_program: account_compression::ID, | ||
| }; | ||
| Instruction { | ||
| program_id: crate::ID, | ||
| accounts: accounts.to_account_metas(Some(true)), | ||
| data: instruction_data.data(), | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding a debug assertion for nodes length.
While compress_proofs already enforces nodes.len() <= NULLIFY_STATE_V1_MULTI_MAX_NODES, direct callers bypassing compression could create oversized transactions that fail at submission with an opaque error. A debug assertion would catch this during development:
debug_assert!(
inputs.nodes.len() <= NULLIFY_STATE_V1_MULTI_MAX_NODES,
"nodes exceeds max: {} > {}",
inputs.nodes.len(),
NULLIFY_STATE_V1_MULTI_MAX_NODES
);This is optional since the failure mode is benign (tx submission fails).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@programs/registry/src/account_compression_cpi/sdk.rs` around lines 101 - 135,
Add a development-time check inside create_nullify_state_v1_multi_instruction to
ensure inputs.nodes.len() does not exceed NULLIFY_STATE_V1_MULTI_MAX_NODES by
inserting a debug_assert that compares inputs.nodes.len() <=
NULLIFY_STATE_V1_MULTI_MAX_NODES and includes a descriptive message like "nodes
exceeds max: {} > {}", referencing inputs.nodes.len() and
NULLIFY_STATE_V1_MULTI_MAX_NODES; this will catch oversized node arrays early
during debugging without affecting release builds.
Contributor
SwenSchaeferjohann
left a comment
SwenSchaeferjohann
left a comment
There was a problem hiding this comment.
devin:
- (Forester): batch_size > 1 can exceed 1232-byte tx limit with ALT
At forester/src/processor/v1/tx_builder.rs:158-172, when ALT is enabled, the code chunks instructions by config.batch_size and packs them into a single v0 transaction.
fix: clamp batch_size to 1
Contributor
|
devin: replace this:
with:
in config.rs:22-25 |
Replace v1-specific ALT with a single unified ALT that includes accounts for all forester operations (v1 state, v1 address, v2 state, v2 address). Solana's v0::Message::try_compile automatically selects relevant entries per instruction, so unused entries cost nothing. - Add ForesterLookupTableParams and forester_lookup_table_accounts() - Create ALT unconditionally in e2e test for all operations - Clamp v1 batch_size to 1 when ALT is present (tx size limit) - Fix min_queue_items doc comment to match actual behavior
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
forester/src/processor/v1/tx_builder.rs (1)
126-129: 🧹 Nitpick | 🔵 TrivialCentralize the queue-size cutoff for multi-nullify.
10_000is duplicated here and inforester/src/processor/v1/send_transaction.rs, so the warning path and the actual gating can drift apart over time. Move it to one shared constant and import it from both sites.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@forester/src/processor/v1/tx_builder.rs` around lines 126 - 129, The hardcoded cutoff 10_000 used to decide use_multi_nullify (const MULTI_NULLIFY_MAX_QUEUE_SIZE) is duplicated elsewhere; create a single shared constant (e.g., pub(crate) MULTI_NULLIFY_QUEUE_CUTOFF) in a common module (or a new constants module) and replace the local const and any duplicate literal usage in forester/src/processor/v1/tx_builder.rs (affecting use_multi_nullify) and forester/src/processor/v1/send_transaction.rs (the warning path) to import and use that shared symbol so both gating and warning use the same value.forester/tests/e2e_test.rs (1)
654-676:⚠️ Potential issue | 🟠 MajorMake missing log artifacts fail this test.
If
logs/or anyforester.*file is missing, this branch only warns and the dedup assertion is skipped, so the test can pass without proving the grouped nullify path ran.Suggested fix
if is_v1_state_test_enabled() { let log_dir = std::path::Path::new("logs"); - if log_dir.exists() { - let latest_log = std::fs::read_dir(log_dir) - .unwrap() - .filter_map(|e| e.ok()) - .filter(|e| e.file_name().to_string_lossy().starts_with("forester.")) - .max_by_key(|e| e.metadata().unwrap().modified().unwrap()); - if let Some(log_entry) = latest_log { - let content = std::fs::read_to_string(log_entry.path()).unwrap(); - let has_dedup = content.contains("v1_nullify_state_v1_multi_grouping"); - assert!( - has_dedup, - "Expected v1_nullify_state_v1_multi_grouping logs when ALT is configured" - ); - println!("Verified: dedup grouping events found in forester logs"); - } else { - println!("Warning: no forester log files found in logs/"); - } - } else { - println!("Warning: logs/ directory not found"); - } + assert!( + log_dir.exists(), + "Expected logs/ directory for dedup-grouping verification" + ); + let latest_log = std::fs::read_dir(log_dir) + .unwrap() + .filter_map(|e| e.ok()) + .filter(|e| e.file_name().to_string_lossy().starts_with("forester.")) + .max_by_key(|e| e.metadata().unwrap().modified().unwrap()) + .expect("Expected at least one forester.* log for dedup-grouping verification"); + let content = std::fs::read_to_string(latest_log.path()).unwrap(); + let has_dedup = content.contains("v1_nullify_state_v1_multi_grouping"); + assert!( + has_dedup, + "Expected v1_nullify_state_v1_multi_grouping logs when ALT is configured" + ); + println!("Verified: dedup grouping events found in forester logs"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@forester/tests/e2e_test.rs` around lines 654 - 676, The test currently only warns when the logs/ directory or any forester.* file is missing, letting the dedup assertion be skipped; change the behavior in the is_v1_state_test_enabled() block so missing artifacts cause the test to fail: replace the println! warnings with panics or asserts (use assert!(log_dir.exists(), "...") for the logs/ directory and assert!(latest_log.is_some(), "...") when no forester.* files are found), keep using latest_log to pick the most recent file, read its content into content and assert that content.contains("v1_nullify_state_v1_multi_grouping") to fail if the dedup logs are absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@forester/tests/e2e_test.rs`:
- Around line 195-196: Update the helper comment above create_forester_alt() to
reflect that it now seeds a unified Forester ALT from ForesterLookupTableParams
covering both v1 and v2 state and address trees (rather than being specific to
nullify_state_v1_multi); mention that it returns the ALT address and that the
ALT is populated with accounts needed across v1/v2 operations so the comment no
longer references only nullify_state_v1_multi.
In `@programs/registry/src/account_compression_cpi/sdk.rs`:
- Around line 135-148: The CompressedProofs docstring is inaccurate: update the
comment for the CompressedProofs struct to match the actual wire/encoding used
by compress_proofs(), stating that nodes are interleaved level-by-level across
all proofs (not grouped per-proof) and that all four entries in proof_bitvecs
(including proof_bitvecs[0]) are used to select nodes for proofs, so proof 1 is
not guaranteed to be nodes[0..16]; also mention the 2–4 proof and unique-node
limits echoed by compress_proofs(). Reference CompressedProofs,
compress_proofs(), proof_bitvecs, and nodes when making the edit.
- Around line 776-792: The test only checks count but must assert the exact
account order matches the runtime's positional expectation; update
test_nullify_state_v1_multi_instruction_accounts to assert ix.accounts (or their
pubkeys) equal the canonical sequence defined by the NullifyLeaves/account order
used by the program (the canonical order referenced in nullify.rs), using the
same identifiers from create_nullify_state_v1_multi_instruction inputs (e.g.,
authority, nullifier_queue, merkle_tree, etc.) so a reordering bug will fail the
test rather than only len() checks.
---
Duplicate comments:
In `@forester/src/processor/v1/tx_builder.rs`:
- Around line 126-129: The hardcoded cutoff 10_000 used to decide
use_multi_nullify (const MULTI_NULLIFY_MAX_QUEUE_SIZE) is duplicated elsewhere;
create a single shared constant (e.g., pub(crate) MULTI_NULLIFY_QUEUE_CUTOFF) in
a common module (or a new constants module) and replace the local const and any
duplicate literal usage in forester/src/processor/v1/tx_builder.rs (affecting
use_multi_nullify) and forester/src/processor/v1/send_transaction.rs (the
warning path) to import and use that shared symbol so both gating and warning
use the same value.
In `@forester/tests/e2e_test.rs`:
- Around line 654-676: The test currently only warns when the logs/ directory or
any forester.* file is missing, letting the dedup assertion be skipped; change
the behavior in the is_v1_state_test_enabled() block so missing artifacts cause
the test to fail: replace the println! warnings with panics or asserts (use
assert!(log_dir.exists(), "...") for the logs/ directory and
assert!(latest_log.is_some(), "...") when no forester.* files are found), keep
using latest_log to pick the most recent file, read its content into content and
assert that content.contains("v1_nullify_state_v1_multi_grouping") to fail if
the dedup logs are absent.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 3f639259-0dda-4d66-84de-f60bb879ab69
📒 Files selected for processing (4)
forester/src/processor/v1/config.rsforester/src/processor/v1/tx_builder.rsforester/tests/e2e_test.rsprograms/registry/src/account_compression_cpi/sdk.rs
Comment on lines
+195
to
+196
| /// Creates an on-chain Address Lookup Table populated with the accounts | ||
| /// needed for nullify_state_v1_multi instructions. Returns the ALT address. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Align the helper comment with the unified ALT behavior.
create_forester_alt() now seeds a unified forester ALT from ForesterLookupTableParams across v1/v2 state and address trees, so the current nullify_state_v1_multi wording is stale.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@forester/tests/e2e_test.rs` around lines 195 - 196, Update the helper comment
above create_forester_alt() to reflect that it now seeds a unified Forester ALT
from ForesterLookupTableParams covering both v1 and v2 state and address trees
(rather than being specific to nullify_state_v1_multi); mention that it returns
the ALT address and that the ALT is populated with accounts needed across v1/v2
operations so the comment no longer references only nullify_state_v1_multi.
Comment on lines
+135
to
+148
| /// Result of compressing 2-4 Merkle proofs into a deduplicated node pool. | ||
| pub struct CompressedProofs { | ||
| /// Bitvecs for proofs 2-4, each selecting 16 nodes from the pool. | ||
| /// proof_1 is always nodes[0..16]. | ||
| pub proof_bitvecs: [u32; 4], | ||
| pub nodes: Vec<[u8; 32]>, | ||
| } | ||
|
|
||
| /// Compresses 2-4 full 16-node Merkle proofs into a deduplicated node pool. | ||
| /// The pool is built level-by-level so that iterating set bits in ascending | ||
| /// order produces nodes in proof-level order. | ||
| /// Proof 1 is always nodes[0..16]. Proofs 2-4 each have a bitvec selecting | ||
| /// which pool nodes form that proof. | ||
| /// Returns `None` if fewer than 2, more than 4 proofs, or too many unique nodes. |
Contributor
There was a problem hiding this comment.
Fix the CompressedProofs docstring to match the encoding.
compress_proofs() interleaves nodes level-by-level across all proofs and also fills proof_bitvecs[0], so proof 1 is not guaranteed to be nodes[0..16]. The current comment describes a different wire format than the implementation.
Suggested fix
/// Result of compressing 2-4 Merkle proofs into a deduplicated node pool.
pub struct CompressedProofs {
- /// Bitvecs for proofs 2-4, each selecting 16 nodes from the pool.
- /// proof_1 is always nodes[0..16].
+ /// Bitvecs for proofs 1-4, each selecting 16 nodes from the pool.
+ /// All proofs, including proof_1, must be reconstructed from their bitvec.
pub proof_bitvecs: [u32; 4],
pub nodes: Vec<[u8; 32]>,
}
/// Compresses 2-4 full 16-node Merkle proofs into a deduplicated node pool.
/// The pool is built level-by-level so that iterating set bits in ascending
/// order produces nodes in proof-level order.
-/// Proof 1 is always nodes[0..16]. Proofs 2-4 each have a bitvec selecting
-/// which pool nodes form that proof.
+/// Each proof, including proof_1, has a bitvec selecting which pool nodes
+/// form that proof.
/// Returns `None` if fewer than 2, more than 4 proofs, or too many unique nodes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@programs/registry/src/account_compression_cpi/sdk.rs` around lines 135 - 148,
The CompressedProofs docstring is inaccurate: update the comment for the
CompressedProofs struct to match the actual wire/encoding used by
compress_proofs(), stating that nodes are interleaved level-by-level across all
proofs (not grouped per-proof) and that all four entries in proof_bitvecs
(including proof_bitvecs[0]) are used to select nodes for proofs, so proof 1 is
not guaranteed to be nodes[0..16]; also mention the 2–4 proof and unique-node
limits echoed by compress_proofs(). Reference CompressedProofs,
compress_proofs(), proof_bitvecs, and nodes when making the edit.
Comment on lines
+776
to
+792
| fn test_nullify_state_v1_multi_instruction_accounts() { | ||
| let authority = Pubkey::new_unique(); | ||
| let inputs = CreateNullifyStateV1MultiInstructionInputs { | ||
| authority, | ||
| nullifier_queue: Pubkey::new_unique(), | ||
| merkle_tree: Pubkey::new_unique(), | ||
| change_log_index: 0, | ||
| queue_indices: [0, 1, 2, 3], | ||
| leaf_indices: [0, 1, 2, 3], | ||
| proof_bitvecs: [0; 4], | ||
| nodes: vec![[0u8; 32]; 16], | ||
| derivation: authority, | ||
| is_metadata_forester: false, | ||
| }; | ||
| let ix = create_nullify_state_v1_multi_instruction(inputs, 0); | ||
| assert_eq!(ix.accounts.len(), 8, "expected 8 accounts"); | ||
| } |
Contributor
There was a problem hiding this comment.
Assert the exact account order here.
NullifyLeaves is positional, and the canonical order in programs/registry/src/account_compression_cpi/nullify.rs:9-30 is what the runtime consumes. A reordering bug would still satisfy len() == 8 and only fail on-chain.
Suggested fix
- let ix = create_nullify_state_v1_multi_instruction(inputs, 0);
- assert_eq!(ix.accounts.len(), 8, "expected 8 accounts");
+ let expected_accounts = vec![
+ get_forester_epoch_pda_from_authority(&inputs.derivation, 0).0,
+ inputs.authority,
+ get_cpi_authority_pda().0,
+ get_registered_program_pda(&crate::ID),
+ account_compression::ID,
+ Pubkey::new_from_array(NOOP_PUBKEY),
+ inputs.merkle_tree,
+ inputs.nullifier_queue,
+ ];
+ let ix = create_nullify_state_v1_multi_instruction(inputs.clone(), 0);
+ let actual_accounts: Vec<Pubkey> = ix.accounts.iter().map(|meta| meta.pubkey).collect();
+ assert_eq!(actual_accounts, expected_accounts, "unexpected account order");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@programs/registry/src/account_compression_cpi/sdk.rs` around lines 776 - 792,
The test only checks count but must assert the exact account order matches the
runtime's positional expectation; update
test_nullify_state_v1_multi_instruction_accounts to assert ix.accounts (or their
pubkeys) equal the canonical sequence defined by the NullifyLeaves/account order
used by the program (the canonical order referenced in nullify.rs), using the
same identifiers from create_nullify_state_v1_multi_instruction inputs (e.g.,
authority, nullifier_queue, merkle_tree, etc.) so a reordering bug will fail the
test rather than only len() checks.
sergeytimoshin
requested changes
Mar 18, 2026
- Implemented `make_get_queue_leaf_indices_body` function to construct the request body for the `getQueueLeafIndices` API. - Added API call for `getQueueLeafIndices` in the photon API module. - Introduced `get_queue_leaf_indices` method in the `TestIndexer` struct to handle the API call. - Updated `LightProgramTest` to include the `get_queue_leaf_indices` method, delegating to the underlying indexer.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
forester/src/processor/v1/send_transaction.rs (1)
364-393:⚠️ Potential issue | 🟡 MinorAvoid synthesizing a default signature when unavailable.
At lines 390-392,
prepared_transaction.signature().unwrap_or_default()creates an all-zero signature when none exists. This pollutes logs and makes failure correlation harder—an all-zero signature looks like a real signature but isn't.🔧 Suggested fix: Keep signature as Option
- let tx_signature = prepared_transaction - .signature() - .unwrap_or_default(); - let tx_signature_str = tx_signature.to_string(); + let tx_signature = prepared_transaction.signature(); + let tx_signature_str = tx_signature + .map(|s| s.to_string()) + .unwrap_or_else(|| "<unsigned>".to_string());Then update the
TransactionSendResultvariants to usetx_signature(which is nowOption<Signature>) directly, avoiding the synthetic default.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@forester/src/processor/v1/send_transaction.rs` around lines 364 - 393, In execute_transaction_chunk_sending, stop synthesizing an all-zero signature by replacing the call prepared_transaction.signature().unwrap_or_default() with keeping the Option returned by prepared_transaction.signature(); propagate that Option<Signature> (do not call to_string on None) into the transaction send flow and update the TransactionSendResult enum variants and any matching/constructing sites to accept Option<Signature> instead of assuming a concrete Signature so you can log/handle None distinctly; ensure all places constructing or matching TransactionSendResult (and any uses of tx_signature_str) are updated to handle the Option properly.forester/src/processor/v1/helpers.rs (1)
438-508:⚠️ Potential issue | 🔴 CriticalCritical: Groups must share the same
change_log_indexto avoid incorrect proofs.At line 469,
change_log_indexis computed from the first item'sroot_seq:let change_log_index = (first_item.1.root_seq % STATE_MERKLE_TREE_CHANGELOG) as u16;However,
group_state_items_for_dedupgroups items purely by compressibility (whether proofs fit within the node limit), not byroot_seq. If items in a group have differentroot_seqvalues, they'll have different changelog indices, but all will be submitted with the first item's index—causing proof verification failures on-chain.🔧 Suggested fix: Partition groups by changelog index
Before building the multi-nullify instruction, ensure all items in a group share the same changelog index. You can either:
Pre-filter in
group_state_items_for_dedupby addingroot_seq % STATE_MERKLE_TREE_CHANGELOGas a grouping key, orPost-filter after grouping by splitting any group with mixed changelog indices:
fn group_state_items_for_dedup( items_with_proofs: &mut [(&WorkItem, MerkleProof)], ) -> Vec<Vec<usize>> { items_with_proofs.sort_by_key(|(_, proof)| proof.leaf_index); let n = items_with_proofs.len(); let mut groups = Vec::new(); let mut i = 0; while i < n { - if i + 4 <= n && try_compress_group(items_with_proofs, i, 4).is_some() { + if i + 4 <= n + && same_changelog_index(items_with_proofs, i, 4) + && try_compress_group(items_with_proofs, i, 4).is_some() + { groups.push((i..i + 4).collect()); i += 4; - } else if i + 3 <= n && try_compress_group(items_with_proofs, i, 3).is_some() { + } else if i + 3 <= n + && same_changelog_index(items_with_proofs, i, 3) + && try_compress_group(items_with_proofs, i, 3).is_some() + { groups.push((i..i + 3).collect()); i += 3; // ... similar for count=2 } else { groups.push(vec![i]); i += 1; } } groups } + +fn same_changelog_index( + items: &[(&WorkItem, MerkleProof)], + start: usize, + count: usize, +) -> bool { + let first_idx = items[start].1.root_seq % STATE_MERKLE_TREE_CHANGELOG; + (start + 1..start + count).all(|i| { + items[i].1.root_seq % STATE_MERKLE_TREE_CHANGELOG == first_idx + }) +}This issue was flagged in a previous review but appears unaddressed in the current implementation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@forester/src/processor/v1/helpers.rs` around lines 438 - 508, The group may contain items with different changelog indices causing invalid proofs; ensure all items used to build a multi-nullify instruction share the same change_log_index (root_seq % STATE_MERKLE_TREE_CHANGELOG). Fix by either updating group_state_items_for_dedup to use (root_seq % STATE_MERKLE_TREE_CHANGELOG) as part of the grouping key, or by post-processing each group before the loop to partition it by that changelog index so that when you compute change_log_index (currently from first_item.1.root_seq) and build CreateNullifyStateV1MultiInstructionInputs you only include items whose (item.1.root_seq % STATE_MERKLE_TREE_CHANGELOG) equals that index.forester/src/epoch_manager.rs (1)
2988-3010:⚠️ Potential issue | 🟠 MajorScope
min_queue_itemsto ALT-enabledStateV1only.Line 3009 currently forwards
self.config.min_queue_itemsfor all V1 processing paths. Sinceprocess_v1is used for bothStateV1andAddressV1, this can throttle AddressV1 (and potentially non-LUT flows) even though this threshold is meant for V1 state nullify dedup.💡 Proposed fix
let batched_tx_config = SendBatchedTransactionsConfig { num_batches: 1, build_transaction_batch_config: BuildTransactionBatchConfig { batch_size: self.config.transaction_config.legacy_ixs_per_tx as u64, compute_unit_price: self.config.transaction_config.priority_fee_microlamports, compute_unit_limit: Some(self.config.transaction_config.cu_limit), enable_priority_fees: self.config.transaction_config.enable_priority_fees, max_concurrent_sends: Some(self.config.transaction_config.max_concurrent_sends), queue_item_count: 0, }, @@ - min_queue_items: self.config.min_queue_items, + min_queue_items: if tree_accounts.tree_type == TreeType::StateV1 + && !self.address_lookup_tables.is_empty() + { + self.config.min_queue_items + } else { + None + }, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@forester/src/epoch_manager.rs` around lines 2988 - 3010, The batched_tx_config currently unconditionally sets min_queue_items from self.config, which incorrectly applies the ALT-only V1 threshold to AddressV1 flows; update the code in the process_v1 path where SendBatchedTransactionsConfig is created so that the min_queue_items field is set only when the V1 processing variant is a StateV1 with ALT enabled (use whatever predicate you already have for ALT-enabled StateV1), and otherwise set min_queue_items to 0 (or the non-ALT default). Specifically change the assignment of min_queue_items in the SendBatchedTransactionsConfig construction (in epoch_manager.rs near the process_v1 handling that builds batched_tx_config) to branch on whether the current V1 is StateV1 (not AddressV1) and ALT is enabled; keep all other fields and config structs (SendBatchedTransactionsConfig, BuildTransactionBatchConfig, RetryConfig) unchanged.
🤖 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 899-907: The get_queue_leaf_indices method currently calls
unimplemented!() which panics; replace that with a non-panicking error return so
callers get an IndexerError instead of a runtime crash — specifically, in the
async fn get_queue_leaf_indices(...) implementation replace
unimplemented!("get_queue_leaf_indices") with an Err(...) returning an
appropriate IndexerError (e.g., an "unsupported/not available" or descriptive
error constructed with the IndexerError variant your crate provides) so
LightProgramTest forwarding of get_queue_leaf_indices returns a Result::Err
rather than panic.
---
Duplicate comments:
In `@forester/src/epoch_manager.rs`:
- Around line 2988-3010: The batched_tx_config currently unconditionally sets
min_queue_items from self.config, which incorrectly applies the ALT-only V1
threshold to AddressV1 flows; update the code in the process_v1 path where
SendBatchedTransactionsConfig is created so that the min_queue_items field is
set only when the V1 processing variant is a StateV1 with ALT enabled (use
whatever predicate you already have for ALT-enabled StateV1), and otherwise set
min_queue_items to 0 (or the non-ALT default). Specifically change the
assignment of min_queue_items in the SendBatchedTransactionsConfig construction
(in epoch_manager.rs near the process_v1 handling that builds batched_tx_config)
to branch on whether the current V1 is StateV1 (not AddressV1) and ALT is
enabled; keep all other fields and config structs
(SendBatchedTransactionsConfig, BuildTransactionBatchConfig, RetryConfig)
unchanged.
In `@forester/src/processor/v1/helpers.rs`:
- Around line 438-508: The group may contain items with different changelog
indices causing invalid proofs; ensure all items used to build a multi-nullify
instruction share the same change_log_index (root_seq %
STATE_MERKLE_TREE_CHANGELOG). Fix by either updating group_state_items_for_dedup
to use (root_seq % STATE_MERKLE_TREE_CHANGELOG) as part of the grouping key, or
by post-processing each group before the loop to partition it by that changelog
index so that when you compute change_log_index (currently from
first_item.1.root_seq) and build CreateNullifyStateV1MultiInstructionInputs you
only include items whose (item.1.root_seq % STATE_MERKLE_TREE_CHANGELOG) equals
that index.
In `@forester/src/processor/v1/send_transaction.rs`:
- Around line 364-393: In execute_transaction_chunk_sending, stop synthesizing
an all-zero signature by replacing the call
prepared_transaction.signature().unwrap_or_default() with keeping the Option
returned by prepared_transaction.signature(); propagate that Option<Signature>
(do not call to_string on None) into the transaction send flow and update the
TransactionSendResult enum variants and any matching/constructing sites to
accept Option<Signature> instead of assuming a concrete Signature so you can
log/handle None distinctly; ensure all places constructing or matching
TransactionSendResult (and any uses of tx_signature_str) are updated to handle
the Option properly.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: f7650653-ef89-49ee-b94a-174fc213bdcf
⛔ Files ignored due to path filters (1)
external/photonis excluded by none and included by none
📒 Files selected for processing (18)
forester/src/epoch_manager.rsforester/src/processor/v1/config.rsforester/src/processor/v1/helpers.rsforester/src/processor/v1/send_transaction.rsforester/src/processor/v1/tx_builder.rsforester/src/queue_helpers.rsforester/src/smart_transaction.rsforester/tests/e2e_test.rssdk-libs/client/src/indexer/indexer_trait.rssdk-libs/client/src/indexer/mod.rssdk-libs/client/src/indexer/photon_indexer.rssdk-libs/client/src/indexer/types/mod.rssdk-libs/client/src/indexer/types/queue.rssdk-libs/client/src/rpc/indexer.rssdk-libs/photon-api/src/codegen.rssdk-libs/photon-api/src/lib.rssdk-libs/program-test/src/indexer/test_indexer.rssdk-libs/program-test/src/program_test/indexer.rs
Comment on lines
+899
to
+907
| async fn get_queue_leaf_indices( | ||
| &self, | ||
| _merkle_tree_pubkey: [u8; 32], | ||
| _limit: u16, | ||
| _start_index: Option<u64>, | ||
| _config: Option<IndexerRpcConfig>, | ||
| ) -> Result<Response<Items<QueueLeafIndex>>, IndexerError> { | ||
| unimplemented!("get_queue_leaf_indices") | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid unimplemented!() in this required Indexer method; it will panic on use.
Line 906 panics. Since LightProgramTest now forwards get_queue_leaf_indices directly, this creates a runtime crash path instead of returning IndexerError.
💡 Suggested implementation
async fn get_queue_leaf_indices(
&self,
- _merkle_tree_pubkey: [u8; 32],
- _limit: u16,
- _start_index: Option<u64>,
- _config: Option<IndexerRpcConfig>,
+ _merkle_tree_pubkey: [u8; 32],
+ _limit: u16,
+ _start_index: Option<u64>,
+ _config: Option<IndexerRpcConfig>,
) -> Result<Response<Items<QueueLeafIndex>>, IndexerError> {
- unimplemented!("get_queue_leaf_indices")
+ let tree = Pubkey::new_from_array(_merkle_tree_pubkey);
+ let start = _start_index.unwrap_or(0) as usize;
+ let limit = _limit as usize;
+
+ let state_tree = self
+ .state_merkle_trees
+ .iter()
+ .find(|x| x.accounts.merkle_tree == tree || x.accounts.nullifier_queue == tree)
+ .ok_or_else(|| IndexerError::InvalidParameters("Merkle tree not found".to_string()))?;
+
+ let items = state_tree
+ .input_leaf_indices
+ .iter()
+ .enumerate()
+ .skip(start)
+ .take(limit)
+ .map(|(queue_index, info)| QueueLeafIndex {
+ hash: info.leaf,
+ queue_index: queue_index as u64,
+ leaf_index: info.leaf_index as u64,
+ })
+ .collect();
+
+ Ok(Response {
+ context: Context {
+ slot: self.get_current_slot(),
+ },
+ value: Items { items },
+ })
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk-libs/program-test/src/indexer/test_indexer.rs` around lines 899 - 907,
The get_queue_leaf_indices method currently calls unimplemented!() which panics;
replace that with a non-panicking error return so callers get an IndexerError
instead of a runtime crash — specifically, in the async fn
get_queue_leaf_indices(...) implementation replace
unimplemented!("get_queue_leaf_indices") with an Err(...) returning an
appropriate IndexerError (e.g., an "unsupported/not available" or descriptive
error constructed with the IndexerError variant your crate provides) so
LightProgramTest forwarding of get_queue_leaf_indices returns a Result::Err
rather than panic.
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
3 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
Forester Configuration for V1 Nullification Deduplication -- Added
min_queue_itemsCLI flag and configuration field to delay processing until queue reaches a threshold (default: 5000), allowing proof deduplication grouping for V1 state nullifications.Address Lookup Table (ALT) Dynamic Management -- Changed
address_lookup_tablesfromArc<Vec<...>>toArc<tokio::sync::RwLock<Vec<...>>>to enable dynamic updates of ALT accounts during epochs, withextend_alt_with_forester_pda()method to register new forester PDAs mid-epoch.New
nullify_dedupInstruction in Registry Program -- Implemented compressed proof encoding/decoding that combines 2-4 Merkle proofs into a single instruction with parametric proof reconstruction (proof_2_shared bitvector, proof_3_source and proof_4_source 2-bit-per-level encoding), reducing transaction size.SDK Helper Functions for Dedup -- Added
create_nullify_dedup_instruction(),compress_proofs(), andnullify_dedup_lookup_table_accounts()utilities to build nullify_dedup instructions with ALT population guidance.Transaction Size Optimization -- Defined
NULLIFY_DEDUP_MAX_NODES = 27constant (verified by tx size test) to fit worst-case 4-leaf nullify_dedup transactions within 1232-byte Solana transaction limit using Address Lookup Tables.Integration Tests for Dedup -- Added comprehensive
test_nullify_dedup.rswith 2-4 leaf scenarios, proof compression validation, and invalid encoding error cases; plus pure serialization testtest_nullify_dedup_tx_size.rsensuring transaction sizes comply with Solana limits.E2E Test Infrastructure for Dedup -- Updated e2e_test to create ALT, configure forester with
min_queue_items, spawn background slot-advancement task (for offline test validator), and validate dedup grouping logs.Error Handling -- Added
InvalidProofEncodingerror variant andcount_from_leaf_indices()helper to validate dedup proof encoding correctness.Test Configuration Updates -- Updated all forester test configs and utilities to include new
min_queue_itemsfield with None defaults.Helper for Slot Advancement -- Added background slot-advancement task in e2e_test to prevent forester from blocking on epoch registration windows in offline validator mode.
Summary by CodeRabbit
New Features
Tests
Documentation / Chores