Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 43614d3. Configure here.
theelderbeever
left a comment
There was a problem hiding this comment.
I don't really have a ton of context but, I added some comments.
| match v { | ||
| 0 => Self::Created, | ||
| 1 => Self::Transferred, | ||
| _ => panic!("invalid InscriptionEventType: {v}"), |
There was a problem hiding this comment.
Is panicking really what you want to do here?
There was a problem hiding this comment.
Ord's Entry trait is infallible. Every existing load() returns Self, not Result, and the whole deserialize() fn here is full of unwraps on try_into() / from_utf8. This explicit panic! is actually the only one with a useful error message; replacing it in isolation doesn't improve safety, and converting to a Result-returning deserialize is a broader change that departs from the rest of the codebase.
| pub fn deserialize(data: &[u8]) -> Self { | ||
| let mut pos = 0; | ||
| let event_type = InscriptionEventType::from_u8(data[pos]); | ||
| pos += 1; | ||
| let block_height = u32::from_le_bytes(data[pos..pos + 4].try_into().unwrap()); |
There was a problem hiding this comment.
Is there a reason for the custom deserialization and storage format?
There was a problem hiding this comment.
no strong reason. The ord-idiomatic approach is the Entry trait with a tuple-typed Value, and the PR could use it here. And we'd probably want to get this into the open-sourced repo in the future so we'd want fewer changes and just follow what's existing currently
| task::block_in_place(|| { | ||
| if !index.has_inscription_event_index() { | ||
| return Err(ServerError::NotFound( | ||
| "this server has no inscription event index".to_string(), | ||
| )); | ||
| } |
There was a problem hiding this comment.
Rationale behind block_in_place? Presumably this is blocking the whole worker thread. What async runtime is being used?
There was a problem hiding this comment.
Runtime is tokio multi-thread (features = ["rt-multi-thread"] in Cargo.toml:96, multi-thread builder at settings.rs:613-623). block_in_place is the established convention for every /r/ handler in this file — there are 22 uses already, going back to the first handler (r::blockhash at line 6). It's specifically designed to prevent worker starvation when calling sync code from an async handler on a multi-threaded runtime: tokio migrates other tasks off the worker before running the closure, so no other task is blocked. For short redb reads like these, it's cheaper than spawn_blocking (no extra thread pool hop). If redb lookups ever grew into genuinely long operations we'd move to spawn_blocking, but that's not today.

https://linear.app/quicknode/issue/DX-4892/track-inscription-creation-and-transfer-events-ordinals-and-runes-api
https://github.com/quiknode-labs/ordinals-json-rpc/pull/7
Summary
INSCRIPTION_EVENT_TO_DATA,SEQUENCE_NUMBER_TO_INSCRIPTION_EVENTS)from_addressandto_addressat index time from script_pubkeys so queries are self-containedGET /r/events/block/{start}/{end}/{page}— query events by block rangeGET /r/events/inscription/{id}/{page}— query full history of an inscription--index-inscription-eventsflag (requires--index-addresses); no behavior change without the flagEvent record fields
event_type,block_height,inscription_id,txid,new_satpoint,old_satpoint,to_address,from_address,charmsFiles changed (9 files, +662 lines)
entry.rsInscriptionEventEntrystruct with binary serialize/deserializeoptions.rs/settings.rs--index-inscription-eventsCLI flagindex.rsStatisticvariant, query methods, integration testsupdater.rsInscriptionUpdaterinscription_updater.rsapi.rsInscriptionEvents/InscriptionEventresponse typesserver.rs/r.rsTest plan
--index-inscription-eventsflag--index-inscription-events --index-addressesDeployment note
Requires full reindex to populate historical events. Recommended: run a parallel instance with the new flags, let it sync from scratch, swap in once caught up.
Note
Medium Risk
Adds new persisted event data and query paths in the indexer, including address derivation and reorg-sensitive writes; issues could affect index correctness and API results when the flag is enabled.
Overview
Adds an optional inscription event index (gated by
--index-inscription-events, requiring--index-addresses) that persists created and transferred events during block indexing, including derivedfrom_address/to_addressand satpoint changes.Introduces new redb tables and query helpers to fetch events by inscription ID or block-height range with pagination, and exposes them via new JSON endpoints
GET /r/events/inscription/{inscription_id}/{page}andGET /r/events/block/{start}/{end}/{page}.Extends the API model with
InscriptionEvents/InscriptionEvent, updates schema/statistics wiring, and adds unit/integration tests covering persistence, flag gating, and reorg rollback behavior.Reviewed by Cursor Bugbot for commit 20842cc. Bugbot is set up for automated code reviews on this repo. Configure here.