Skip to content

fix(query): substrate events emit "callAddress" twice#82

Open
mo4islona wants to merge 1 commit into
masterfrom
fix/substrate-duplicate-call-address
Open

fix(query): substrate events emit "callAddress" twice#82
mo4islona wants to merge 1 commit into
masterfrom
fix/substrate-duplicate-call-address

Conversation

@mo4islona

@mo4islona mo4islona commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

The defect

EventFieldSelection's projection in crates/query/src/query/substrate.rs lists this.call_address at two positions:

project(this) json_object! {{
    this.index,
    this.extrinsic_index,
    this.call_address,   // ← 135
    this.name,
    this.phase,
    this.call_address,   // ← 138
    this.topics,
    [this.args]: Json,
}}

Nothing downstream deduplicates: JsonObject::add pushes onto a Vec, eval_object walks it, StructEncoder writes every field. So selecting callAddress emits the property twice. Raw bytes off fixtures/moonbeam/chunk:

"events":[{"index":0,"callAddress":null,"name":"Treasury.UpdatedInactive","callAddress":null},…]

264 occurrences of "callAddress" across 132 event objects — exactly 2:1.

RFC 8259 §4 says object names SHOULD be unique. Parsers that accept a duplicate keep one of the two. Both props read the same column, so the values agree and no consumer ever observed a wrong value. The cost is dead bytes on the wire and a response a strict parser is not obliged to accept.

Introduced in 9fcb79d (2024-11-14).

Why no existing test caught it

test_fixture runs both sides through serde_json::from_slice. serde_json's map keeps one entry per key, so the duplicate collapses before any assertion sees it. The committed result.json files were produced the same way and contain one callAddress per event. The suite is structurally blind to this class of defect.

The change

One line removed (substrate.rs), plus tests that would have caught it (tests/fixtures.rs).

I removed the second occurrence, not the first. Parsers keep the first occurrence's position (with the last value), so the parsed key order stays exactly what consumers see today. The change is invisible to any JSON client — and indeed none of the 116 fixture result.json files needed updating, which is the proof.

Two layers of test

1. substrate_event_emits_call_address_once — a named regression test. Runs a literal query against the moonbeam fixture chunk, asserts on the raw response bytes, then asserts the query actually returned events and that each carries the property under test, so it cannot pass vacuously. It does not depend on any fixture's query.json continuing to select the field.

2. assert_unique_keys — a corpus-wide check wired into test_fixture, so every fixture in both the parquet and storage backends now fails on any object that repeats a property name. Generic, not substrate-specific.

Both use a small Deserialize impl rather than serde_json::Value, because Value is precisely the thing that hides the bug.

Verification

Red. Re-introducing the duplicate projection entry, with the new named test in place:

thread 'parquet::substrate_event_emits_call_address_once' panicked at crates/query/tests/fixtures.rs:82:9:
substrate_event_emits_call_address_once: duplicate property "callAddress" at line 1 column 107
test result: FAILED. 0 passed; 1 failed

And the corpus-wide check, against the tree before the fix:

test result: FAILED. 98 passed; 18 failed
    …kusama_queries_example_balances_squid… duplicate property "callAddress"
    …moonbeam_queries_simple_event_query…   duplicate property "callAddress"
    (16 more, all substrate)

All 18 failures are the substrate fixtures that select callAddress; the other 98 pass. So no other projection in the corpus has this defect — I also confirmed that statically by scanning every project(this) json_object! block for a repeated this.X, and substrate's is the only one.

Green. After the fix:

test result: ok. 117 passed; 0 failed

Formatted with the repo's pinned toolchain (1.89) and rustfmt.toml; substrate.rs is a clean one-line deletion with no formatting churn.

Context

Found while writing an implementation-independent specification of the query engine and diffing it against a reimplementation. Reporting it here since this is where the fix belongs.

🤖 Generated with Claude Code

`EventFieldSelection`'s projection lists `this.call_address` at two
positions, so selecting `callAddress` adds two props with the same name.
Nothing downstream deduplicates them: `JsonObject::add` pushes onto a
`Vec`, `eval_object` walks it, and `StructEncoder` writes every field.

Raw response, before this change:

    "events":[{"index":0,"callAddress":null,"name":"Treasury.UpdatedInactive","callAddress":null}]

RFC 8259 s4 says object names SHOULD be unique; parsers that accept a
duplicate keep one of the two. Both props read the same column, so the
values agree and no consumer observes a wrong value — but the response
carries dead bytes and is not something a strict parser must accept.

Removes the *second* occurrence rather than the first. Parsers keep the
first occurrence's position, so the parsed key order is exactly what
consumers already see today. This change is therefore invisible to any
JSON client, and none of the 116 fixture results needed updating.

Introduced in 9fcb79d (2024-11-14).

Why no test caught it: `test_fixture` runs both sides through
`serde_json::from_slice`, and `serde_json`'s map keeps one entry per key,
so the duplicate is collapsed before any assertion sees it. The committed
`result.json` files were produced the same way and contain one
`callAddress` per event. The suite is structurally blind to this class of
defect.

So this adds two layers of test:

  * `substrate_event_emits_call_address_once`, a named regression test.
    It runs a literal query against the moonbeam fixture chunk, asserts on
    the raw response bytes, then asserts the query actually returned events
    and that each one carries the property under test — so it cannot pass
    vacuously, and it does not depend on any fixture's `query.json`
    continuing to select the field. `execute_query` is split into
    `execute_query_bytes` so a query can run without a file on disk.

  * `assert_unique_keys`, wired into `test_fixture`, so every fixture in
    both the parquet and storage backends now fails on any object that
    repeats a property name. Generic, not substrate-specific.

Both use a small `Deserialize` impl rather than `serde_json::Value`,
because `Value` is precisely the thing that hides the bug.

Verified red and green. Re-introducing the duplicate projection entry
makes the named test fail with `duplicate property "callAddress"`, and
makes the corpus-wide check fail on all 18 substrate fixtures that select
the field while the other 98 pass — so no other projection in the corpus
has this defect. Removing it again makes all 117 tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mo4islona mo4islona force-pushed the fix/substrate-duplicate-call-address branch from 4e58e74 to e06778f Compare July 9, 2026 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants