Interpreter fixes: Ore stack simplification, resolver improvements, and IDL parsing#60
Merged
Interpreter fixes: Ore stack simplification, resolver improvements, and IDL parsing#60
Conversation
Fixes three bugs introduced after the Scheduled URL Resolver merge that broke cross-account field mapping (lookup_index with register_from) at round boundaries: 1. Compiler key resolution priority swap — resolved_key_reg (PDA reverse lookup result) was given priority over the LookupIndex result, causing mutations to be keyed by round_address instead of round_id. 2. Null-key handler execution — when LookupIndex returned null, handlers continued with key=null creating phantom entities that prevented queueing for reprocessing. Added AbortIfNullKey opcode. 3. Stale lookup index entries — PDA mapping changes at round boundaries left old entries that shadowed the updated mapping. Added PDA change detection, stale index clearing, and last_account_data cache for reprocessing.
- Add Keccak256 computed expression for cryptographic hashing - Implement slot_hash_bytes and expires_at_slot_hash fields in Ore stack - Add pre_reveal_rng and pre_reveal_winning_square computed fields - Add sha3 dependency to hyperstack runtime features
- Add skip_resolvers flag to UpdateContext for reprocessed cached data - Create UpdateContext::new_reprocessed() for PDA mapping change scenarios - Add is_stale_reprocess flag to PendingAccountUpdate - Skip QueueResolver opcodes when processing stale reprocessed data - Add slot_hash_cache module for efficient slot hash lookups
…nced logging - Add slot tracker notification support with 5s polling fallback - Export generate_computed_expr_code_with_cache for intra-section caching - Add detailed tracing for scheduler callback processing - Improve condition evaluation visibility with field value logging - Add SetOnce guard logging to track skipped callbacks
- Add yellowstone-grpc-client and yellowstone-grpc-proto dependencies - Update Ore React components with latest schema changes - Update Ore TypeScript example with local development URL - Update hyperstack-server with new health check improvements - Sync all Cargo.lock files with new dependencies
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR is a substantial second iteration that resolves the majority of issues raised in the previous review cycle. It delivers Ore stack slot-hash infrastructure, resolver improvements, IDL parsing fixes, and a reworked What changed
Issues found
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant GeyserGRPC as Yellowstone gRPC
participant SlotSubTask as Slot Subscription Task
participant SlotTracker as SlotTracker
participant SlotHashCache as slot_hash_cache (global)
participant SlotScheduler as Slot Scheduler Task
participant VM as VmContext
participant Resolver as SlotHashResolver
GeyserGRPC->>SlotSubTask: SlotUpdate(slot)
SlotSubTask->>SlotTracker: record(slot)
SlotTracker->>SlotTracker: notify_waiters() ⚠️ (may miss if no waiter registered)
SlotTracker-->>SlotScheduler: notified() OR 5s timeout
GeyserGRPC->>SlotSubTask: AccountUpdate(SlotHashes sysvar)
SlotSubTask->>SlotHashCache: record_slot_hash(slot, hash) × 512
SlotScheduler->>SlotTracker: get() → current_slot
SlotScheduler->>VM: process_event (due callbacks)
Note over VM,Resolver: Computed field resolution
VM->>Resolver: evaluate_computed("slot_hash", [slot])
Resolver->>SlotHashCache: get_slot_hash(slot)
SlotHashCache-->>Resolver: base58 hash string
Resolver-->>VM: Value::Object { bytes: [...] }
VM->>Resolver: evaluate_computed("keccak_rng", [hash, seed, samples])
Resolver-->>VM: Value::String (u64 XOR-fold as decimal string)
Prompt To Fix All With AIThis is a comment left during a code review.
Path: rust/hyperstack-server/src/health.rs
Line: 34-36
Comment:
**`notify_waiters()` can miss notifications under load**
`notify_waiters()` only wakes tasks that are *currently registered* as waiters (i.e., already awaiting `notified()`). If `record()` fires while the scheduler loop is between iterations — processing callbacks and not yet suspended in the `tokio::select!` — the notification is silently dropped and the scheduler must wait the full 5-second fallback timeout.
For Solana's ~400 ms slot cadence this means time-sensitive callbacks (e.g. Ore round-expiry) can be up to 5 seconds late in the worst case. `notify_one()` stores a permit and guarantees the *next* `notified()` call resolves immediately regardless of when the wakeup arrives:
```rust
pub fn record(&self, slot: u64) {
let old = self.last_slot.fetch_max(slot, Ordering::Relaxed);
if slot > old {
self.notify.notify_one(); // stores a permit — never missed
}
}
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: rust/hyperstack-server/src/health.rs
Line: 74-96
Comment:
**Dead code: `GLOBAL_SLOT_TRACKER` and related functions are never used**
`init_global_slot_tracker`, `GLOBAL_SLOT_TRACKER`, and the module-level `get_slot_hash` function are added in this PR but are never called from anywhere else in the codebase (confirmed by search). The actual slot-hash storage used by the resolvers is `interpreter/src/slot_hash_cache.rs` — the generated code calls `hyperstack_interpreter::record_slot_hash` and `SlotHashResolver::evaluate_slot_hash` reads from `crate::slot_hash_cache::get_slot_hash`, bypassing this infrastructure entirely.
Similarly, `SlotTracker::record_slot_hash` and `SlotTracker::get_slot_hash` are never called from the generated code. Leaving these in creates confusion about which cache is authoritative and adds maintenance surface. Consider either:
- Wiring up `init_global_slot_tracker` so this infrastructure is actually used, or
- Removing it until it is needed.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: rust/hyperstack-server/src/health.rs
Line: 39-52
Comment:
**`record_slot_hash` holds the write lock during O(n) full-map traversal**
The pruning logic holds the `RwLock` write lock while iterating every key to find stale entries, then removes them one by one. This approach acquires the lock, scans all entries, collects pruning candidates, and removes them — all while blocking every reader. For a `HashMap` keyed by slot, a more efficient approach is to track the watermark separately (since `slot` is monotonically increasing) so that entries older than `slot - 10000` can be removed without a full scan:
```rust
// Only needs to check entries older than the watermark; in practice
// the collect+remove pattern is already the best option for HashMap,
// but holding the lock only after computing the removal list would help:
let threshold = slot.saturating_sub(10_000);
hashes.retain(|&s, _| s >= threshold);
```
Note: this function appears to be unreachable dead code currently (see the `GLOBAL_SLOT_TRACKER` comment above), so fixing the performance here is secondary to deciding whether this code path will ever be used.
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: "fix: Keccak types" |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
The condition `!event_type.ends_with(CpiEvent)` was logically redundant since the outer check `event_type.ends_with(State)` already guarantees the string cannot end with CpiEvent.
- Add missing fields to IdlAccountSnapshot test initialization - Implement automatic discriminant size detection based on instruction format - Add address field to ore.json fixture for IdlSnapshot compatibility - Remove non-standard discriminant_size field from ore.json
- Remove unused imports in hyperstack-macros and hyperstack-server - Fix empty else branch in vm.rs - Remove unused variables in scheduler.rs and vm.rs - Fix let-and-return pattern in VmContext::new_multi_entity() - Add #[allow(clippy::type_complexity)] for complex entity_evaluator type
…esolver output types The `pre_reveal_rng` field (computed via `keccak_rng` resolver) was being typed as `number` instead of `string`. The resolver returns `Value::String` to avoid JavaScript precision loss for large u64 values. Changes: - Updated `is_output_type()` to check TypeScript interfaces for type aliases - Fixed `add_unmapped_fields()` to use resolver types from field_mappings - Updated `uses_builtin_type()` to check field_mappings for computed fields This ensures computed fields that use resolver methods get the correct TypeScript types generated.
…t emitter - Map `slot_hash` resolver method to `SlotHashBytes` (not `SlotHash`) so the interface and Zod schema names match what the resolver actually declares - Extend `is_output_type()` to recognise types declared via `export interface` in addition to type aliases, so `SlotHashBytes` is found as a builtin type - Extend `generate_builtin_resolver_interfaces/schemas` to emit the full resolver block whenever any type it declares is used (not just the primary output_type), covering the `KeccakRngValue`/`SlotHashBytes` split - Filter the schemas const-block to only include `*Schema`-named entries in both the single-entity and multi-entity code paths, preventing bare group names like `SlotHashTypes` from leaking into the stack object
…uctural SourceSpec field
Comment on lines
+34
to
+36
| self.notify.notify_waiters(); | ||
| } | ||
| } |
There was a problem hiding this comment.
notify_waiters() can miss notifications under load
notify_waiters() only wakes tasks that are currently registered as waiters (i.e., already awaiting notified()). If record() fires while the scheduler loop is between iterations — processing callbacks and not yet suspended in the tokio::select! — the notification is silently dropped and the scheduler must wait the full 5-second fallback timeout.
For Solana's ~400 ms slot cadence this means time-sensitive callbacks (e.g. Ore round-expiry) can be up to 5 seconds late in the worst case. notify_one() stores a permit and guarantees the next notified() call resolves immediately regardless of when the wakeup arrives:
pub fn record(&self, slot: u64) {
let old = self.last_slot.fetch_max(slot, Ordering::Relaxed);
if slot > old {
self.notify.notify_one(); // stores a permit — never missed
}
}Prompt To Fix With AI
This is a comment left during a code review.
Path: rust/hyperstack-server/src/health.rs
Line: 34-36
Comment:
**`notify_waiters()` can miss notifications under load**
`notify_waiters()` only wakes tasks that are *currently registered* as waiters (i.e., already awaiting `notified()`). If `record()` fires while the scheduler loop is between iterations — processing callbacks and not yet suspended in the `tokio::select!` — the notification is silently dropped and the scheduler must wait the full 5-second fallback timeout.
For Solana's ~400 ms slot cadence this means time-sensitive callbacks (e.g. Ore round-expiry) can be up to 5 seconds late in the worst case. `notify_one()` stores a permit and guarantees the *next* `notified()` call resolves immediately regardless of when the wakeup arrives:
```rust
pub fn record(&self, slot: u64) {
let old = self.last_slot.fetch_max(slot, Ordering::Relaxed);
if slot > old {
self.notify.notify_one(); // stores a permit — never missed
}
}
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+39
to
+52
| pub async fn record_slot_hash(&self, slot: u64, slot_hash: String) { | ||
| let mut hashes = self.slot_hashes.write().await; | ||
| hashes.insert(slot, slot_hash); | ||
|
|
||
| // Prune old entries to prevent unbounded growth (keep last 10000 slots) | ||
| let slots_to_remove: Vec<u64> = hashes | ||
| .keys() | ||
| .filter(|&&s| s < slot.saturating_sub(10000)) | ||
| .copied() | ||
| .collect(); | ||
| for s in slots_to_remove { | ||
| hashes.remove(&s); | ||
| } | ||
| } |
There was a problem hiding this comment.
record_slot_hash holds the write lock during O(n) full-map traversal
The pruning logic holds the RwLock write lock while iterating every key to find stale entries, then removes them one by one. This approach acquires the lock, scans all entries, collects pruning candidates, and removes them — all while blocking every reader. For a HashMap keyed by slot, a more efficient approach is to track the watermark separately (since slot is monotonically increasing) so that entries older than slot - 10000 can be removed without a full scan:
// Only needs to check entries older than the watermark; in practice
// the collect+remove pattern is already the best option for HashMap,
// but holding the lock only after computing the removal list would help:
let threshold = slot.saturating_sub(10_000);
hashes.retain(|&s, _| s >= threshold);Note: this function appears to be unreachable dead code currently (see the GLOBAL_SLOT_TRACKER comment above), so fixing the performance here is secondary to deciding whether this code path will ever be used.
Prompt To Fix With AI
This is a comment left during a code review.
Path: rust/hyperstack-server/src/health.rs
Line: 39-52
Comment:
**`record_slot_hash` holds the write lock during O(n) full-map traversal**
The pruning logic holds the `RwLock` write lock while iterating every key to find stale entries, then removes them one by one. This approach acquires the lock, scans all entries, collects pruning candidates, and removes them — all while blocking every reader. For a `HashMap` keyed by slot, a more efficient approach is to track the watermark separately (since `slot` is monotonically increasing) so that entries older than `slot - 10000` can be removed without a full scan:
```rust
// Only needs to check entries older than the watermark; in practice
// the collect+remove pattern is already the best option for HashMap,
// but holding the lock only after computing the removal list would help:
let threshold = slot.saturating_sub(10_000);
hashes.retain(|&s, _| s >= threshold);
```
Note: this function appears to be unreachable dead code currently (see the `GLOBAL_SLOT_TRACKER` comment above), so fixing the performance here is secondary to deciding whether this code path will ever be used.
How can I resolve this? If you propose a fix, please make it concise.
adiman9
added a commit
that referenced
this pull request
Mar 25, 2026
Interpreter fixes: Ore stack simplification, resolver improvements, and IDL parsing
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
1 participant
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
This PR introduces significant improvements to the interpreter and resolver system, particularly around the Ore stack implementation.
Key Changes
Ore Stack Simplification
keccak_rng- a working name for now)Resolver System Fixes
skip_resolversmechanism for stale data reprocessingCore Interpreter Improvements
rt-multi-threadfeature supportIDL Parsing
Cleanup
Testing
Note: While
keccak_rngworks as an internal name, it's acknowledged as temporary.