Per-row transaction field selection in store materialization - #1358
Conversation
Replace the chain-global transaction-field union mask with a per-event mask carried on eventConfig. materialize now takes a per-row mask column (block numbers, transaction indices, masks) in one napi call, so each transaction decodes only the fields the events on it selected. A large field such as input is materialized only on the rows that asked for it, which avoids pulling it into the JS buffer for contract-register items that never read it. - transaction_store.rs: materialize takes masks: Vec<f64>; the union decides which columns exist and fill_masked yields None (skipping the decode) for rows whose mask lacks the field. - eventConfig gains transactionFieldMask, computed once at config build; materializeItems reads it per item and ORs masks per (block, txIndex). - Drop the now-dead Ecosystem.transactionFieldMask, makeMaskFn array form and ChainState.transactionFieldMask. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SbnkNd2oW3HbweJv9jdTBG
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughTransaction field masking moves from a single per-batch value held on ChangesPer-row transaction field masking
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
materializePageItems only uses the page store, not the chain state, so remove the parameter (and the now-unused chainState arg threaded through runContractRegistersOrThrow). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SbnkNd2oW3HbweJv9jdTBG
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/envio/src/sources/TransactionStore.res (1)
25-33: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFail fast when a selected field has no mask code.
Line 30 currently drops unmapped field names on the floor. Because these sets come from typed config and hand-maintained ordered field arrays, that turns a contract drift into a partially materialized transaction instead of a loud failure.
Possible fix
selectedTransactionFields->Utils.Set.forEach(name => switch codes->Utils.Dict.dangerouslyGetNonOption(name) { | Some(code) => mask := mask.contents +. pow2(code) - | None => () + | None => + JsError.throwWithMessage( + `Unknown transaction field '${name}' while building transactionFieldMask`, + ) } )🤖 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 `@packages/envio/src/sources/TransactionStore.res` around lines 25 - 33, The maskFromFields helper currently ignores selected field names that are missing from codes, which hides config drift and produces partial transactions. Update maskFromFields to fail fast when Utils.Dict.dangerouslyGetNonOption(name) returns None, and surface a clear error instead of silently skipping the field. Keep the change localized to maskFromFields and its call path so any unmapped selectedTransactionFields entry triggers an immediate failure.
🤖 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 `@packages/cli/src/transaction_store.rs`:
- Line 740: The materialize entry point in transaction_store.rs is blindly
coercing incoming mask values with the masks-to-u64 conversion, which can
silently accept NaN, Infinity, fractions, negatives, or out-of-range JS numbers.
Update the N-API boundary validation in materialize so only valid integer 32-bit
bitmasks are accepted before decoding, and reject invalid inputs early rather
than casting them in the masks conversion path.
In `@packages/envio/src/sources/TransactionStore.res`:
- Around line 103-110: The grouping logic in TransactionStore.res is merging
event masks with orMask and then applying that union to every payload in the
same transaction group, which causes per-event field leakage. Update the
transaction grouping flow so each event keeps its own mask when building
payloads, and avoid stamping a unionized mask across the whole group in the
finalize path after payloadGroups accumulation. Use the existing
TransactionStore grouping code around payloadGroups, masks, and the later
payload assembly logic to ensure fields like input and accountKeys are only
fetched for the specific event that requested them.
---
Nitpick comments:
In `@packages/envio/src/sources/TransactionStore.res`:
- Around line 25-33: The maskFromFields helper currently ignores selected field
names that are missing from codes, which hides config drift and produces partial
transactions. Update maskFromFields to fail fast when
Utils.Dict.dangerouslyGetNonOption(name) returns None, and surface a clear error
instead of silently skipping the field. Keep the change localized to
maskFromFields and its call path so any unmapped selectedTransactionFields entry
triggers an immediate failure.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 05880da7-f7d3-4d90-8f4b-58c5edbae90f
📒 Files selected for processing (16)
packages/cli/src/transaction_store.rspackages/envio/src/ChainState.respackages/envio/src/ChainState.resipackages/envio/src/Ecosystem.respackages/envio/src/EventConfigBuilder.respackages/envio/src/Internal.respackages/envio/src/sources/Evm.respackages/envio/src/sources/Fuel.respackages/envio/src/sources/Svm.respackages/envio/src/sources/TransactionStore.resscenarios/fuel_test/test/HyperFuelSource_test.resscenarios/test_codegen/test/EventRouter_svm_test.resscenarios/test_codegen/test/SourceBlockHashes_test.resscenarios/test_codegen/test/SvmHyperSyncSource_test.resscenarios/test_codegen/test/TransactionStore_test.resscenarios/test_codegen/test/helpers/MockIndexer.res
💤 Files with no reviewable changes (3)
- packages/envio/src/ChainState.resi
- packages/envio/src/sources/Fuel.res
- packages/envio/src/Ecosystem.res
| ))); | ||
| } | ||
| let mask = mask as u64; | ||
| let masks: Vec<u64> = masks.iter().map(|&m| m as u64).collect(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the N-API mask boundary and upstream 32-bit mask contract.
rg -n -C3 'masks: Vec<f64>|m as u64|maskFromFields|orMask|transactionFieldMask' \
packages/cli/src/transaction_store.rs \
packages/envio/src/sources/TransactionStore.res \
packages/envio/src/Internal.resRepository: enviodev/hyperindex
Length of output: 6104
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the Rust binding and nearby mask handling to judge scope and impact.
sed -n '680,790p' packages/cli/src/transaction_store.rs
printf '\n----\n'
sed -n '1,140p' packages/envio/src/sources/TransactionStore.res
printf '\n----\n'
sed -n '410,440p' packages/envio/src/Internal.resRepository: enviodev/hyperindex
Length of output: 12316
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether the Rust method is publicly exported through the JS/TS layer and whether any validation exists elsewhere.
rg -n -C2 'materialize\(|transactionFieldMask|array<float>|from_reason|is_finite|fract\(\)|u32::MAX' \
packages/cli/src/transaction_store.rs \
packages/envio/src/sources/TransactionStore.res \
packages/envio/src/Internal.res \
packages/envio/src \
packages/cli/srcRepository: enviodev/hyperindex
Length of output: 18822
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether `materialize` is only called from trusted internal code or exposed more broadly.
rg -n -C3 '\bmaterialize\b|\btransactionFieldMask\b|array<float>' \
packages/envio/src \
packages/cli/srcRepository: enviodev/hyperindex
Length of output: 9389
Reject non-integer masks at the N-API boundary. The current callers build 32-bit bitmasks, but this public materialize entry point still accepts arbitrary JS numbers; m as u64 will silently coerce NaN, Infinity, fractions, negatives, and out-of-range values into a different mask. Reject invalid inputs or narrow the type before decoding.
🤖 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 `@packages/cli/src/transaction_store.rs` at line 740, The materialize entry
point in transaction_store.rs is blindly coercing incoming mask values with the
masks-to-u64 conversion, which can silently accept NaN, Infinity, fractions,
negatives, or out-of-range JS numbers. Update the N-API boundary validation in
materialize so only valid integer 32-bit bitmasks are accepted before decoding,
and reject invalid inputs early rather than casting them in the masks conversion
path.
| let last = payloadGroups->Array.length - 1 | ||
| if ( | ||
| last >= 0 && | ||
| blockNumbers->Array.getUnsafe(last) == blockNumber && | ||
| transactionIndices->Array.getUnsafe(last) == transactionIndex | ||
| ) { | ||
| payloadGroups->Array.getUnsafe(last)->Array.push(eventItem.payload) | ||
| masks->Array.setUnsafe(last, orMask(masks->Array.getUnsafe(last), mask)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Grouping by transaction and OR-ing masks leaks fields across events.
Line 110 unions different event masks, and Lines 124-127 then stamp that unionized transaction onto every payload in the group. An event that selected only cheap fields will still receive input/accountKeys if another event on the same transaction asked for them, which breaks per-event field selection and weakens the PR’s performance goal.
Possible fix
- // Deduped per (blockNumber, transactionIndex); each row's mask is the OR of the
- // masks of the events sharing that transaction.
+ // Deduped only when both the transaction key and selected mask match, so each
+ // payload still receives exactly the fields its own event selected.
@@
if (
last >= 0 &&
blockNumbers->Array.getUnsafe(last) == blockNumber &&
- transactionIndices->Array.getUnsafe(last) == transactionIndex
+ transactionIndices->Array.getUnsafe(last) == transactionIndex &&
+ masks->Array.getUnsafe(last) == mask
) {
payloadGroups->Array.getUnsafe(last)->Array.push(eventItem.payload)
- masks->Array.setUnsafe(last, orMask(masks->Array.getUnsafe(last), mask))
} else {
blockNumbers->Array.push(blockNumber)
transactionIndices->Array.push(transactionIndex)
masks->Array.push(mask)
payloadGroups->Array.push([eventItem.payload])Also applies to: 124-127
🤖 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 `@packages/envio/src/sources/TransactionStore.res` around lines 103 - 110, The
grouping logic in TransactionStore.res is merging event masks with orMask and
then applying that union to every payload in the same transaction group, which
causes per-event field leakage. Update the transaction grouping flow so each
event keeps its own mask when building payloads, and avoid stamping a unionized
mask across the whole group in the finalize path after payloadGroups
accumulation. Use the existing TransactionStore grouping code around
payloadGroups, masks, and the later payload assembly logic to ensure fields like
input and accountKeys are only fetched for the specific event that requested
them.
- Add a ReScript test for orMask (unsigned 32-bit OR incl. the bit-31 edge) and a grouping test for adjacent same-tx events with differing masks. - Hoist the duplicated Set.fromArray/magic cast to a local in the EVM and SVM test config helpers so the field set is built once. - Reword the maskFromFields and materialize doc comments that claimed masks avoid 32-bit bitwise ops, now that orMask uses them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SbnkNd2oW3HbweJv9jdTBG
2 participants
Summary
Changes transaction field materialization from a single ecosystem-wide bitmask to per-row masks, allowing each event to decode only the transaction fields it selected. This avoids materializing expensive fields (e.g.,
input,accountKeys) for events that didn't request them.Key Changes
Rust CLI (
transaction_store.rs):fill()tofill_masked()and added per-row mask support; fields are now materialized only on rows whose mask has the field's bit setdecode_evm_columns()anddecode_svm_columns()to acceptmasks: &[u64]instead of a singlemask: u64; compute the union of all masks to determine which columns to build, but apply each row's own mask when populating cellsmaterialize()method signature to acceptmasks: Vec<f64>with per-row validationdecode_applies_each_rows_own_mask()verifying per-row masking behaviorReScript runtime (
TransactionStore.res):mask()tomaskFromFields()and changed signature to accept a single event'sselectedTransactionFieldsinstead of all event configsmakeMaskFn()to build a per-event mask function (closed over field codes)orMask()helper to bitwise-OR two masks (handles unsigned conversion for bit 31+)materialize()external binding to acceptmasks: array<float>instead of singlemask: floatmaterializeItems()to:maskparameter; read each event'seventConfig.transactionFieldMaskinstead(blockNumber, transactionIndex)and OR their masks togethermaterialize()callConfig and type updates:
transactionFieldMask: floatfield toInternal.eventConfig(precomputed per event)transactionFieldMaskfromChainState.t(no longer ecosystem-wide)EventConfigBuilderto compute and cache each event's mask viaEvm.eventTransactionFieldMask()orSvm.eventTransactionFieldMask()transactionFieldMaskfromEcosystem.t(no longer needed)transactionFieldMaskfield on event configsNotable Implementation Details
(blockNumber, transactionIndex)key, so a transaction decodes the union of fields requested by its eventsNonefor unselected fieldsinputandaccountKeysare now only touched (hex-encoded, cloned) on rows that explicitly selected them, improving performance for batches with mixed field selectionstransactionIndexis always available from the store key regardless of mask, so it resolves correctly even when a row selects no other fieldshttps://claude.ai/code/session_01SbnkNd2oW3HbweJv9jdTBG
Summary by CodeRabbit
New Features
Bug Fixes
Refactor