Skip to content

ingest/contractevents: tolerate CAP-86 sparse maps in V4 SAC event data - #223

Closed
tamirms wants to merge 2 commits into
mainfrom
cap-86-sparse-udt-maps
Closed

ingest/contractevents: tolerate CAP-86 sparse maps in V4 SAC event data#223
tamirms wants to merge 2 commits into
mainfrom
cap-86-sparse-udt-maps

Conversation

@tamirms

@tamirms tamirms commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Companion to go-stellar-sdk#5983, which has the full write-up of the encoding change.

What

CAP-0086 (protocol 28) adds host functions that build Soroban UDT struct maps sparsely: a field holding None is omitted from the map rather than stored bound to Void. Both encodings will exist in ledger history forever, so a decoder must treat key absent and key bound to Void as the same thing and must not assume a fixed map arity.

parseSacEventMap assumed both:

if len(mapData) != 2 { ... }          // exact arity
...
} else if !foundMuxedId { ... }        // to_muxed_id mandatory

A transfer with no muxed destination cannot satisfy either, in either encoding.

Changes

  • Require only amount.
  • Accept to_muxed_id bound to Void as "no muxed destination" (it decodes to MemoNone, which both consumers already handle explicitly).
  • Ignore unrecognized keys, so a field added to the event data in a later protocol cannot take these records down with it.
  • makeV4MapData now builds a map without a to_muxed_id key for a MemoNone memo, and a new makeV4MapDataWithMuxedID covers the explicit-Void shape. Tests cover both encodings of None plus the unknown-key case.

Why it matters, and what it does not change

This is forward-compat hardening, not a fix for anything in ledger history. Verified against rs-soroban-env: the Stellar Asset Contract emits a map only when a muxed destination exists, and always with both keys — so no event on chain today hits the rejected shapes, and no reingestion is needed. The accepted set is unchanged for all existing data.

It is worth doing anyway because of how callers treat a parse failure. Every one of them discards the event rather than surfacing the error:

  • effects_processor.go:1547if err != nil { continue }
  • operations_processor.go:793, :1096if ... err == nil

So a shape this parser rejects does not raise an ingestion error; it silently costs that event its account_credited/account_debited effects, its balance_changes operation details, and its participants — leaving the transaction unindexed for the accounts involved. Failing open is the correct bias here, which is also why unrecognized keys are now ignored rather than rejected.

Note for review

Two test expectations flip, both intentionally:

  • V4 map data insufficient elements → renamed V4 map data with no entries; the error is now amount field not found in map rather than expected exactly 2 elements in map data.
  • V4 map data - missing muxed id → renamed V4 map data with an unrecognized key; it now parses successfully instead of erroring. (Its fixture carries a typo'd tooo_muxed_id key, so it was really testing unknown-key handling all along.)

Deliberately self-contained

The SDK PR adds xdr.ScMap.GetSymbolField, which encodes this rule once. This PR does not use it, so that it compiles against the currently pinned go-stellar-sdk. Worth a follow-up to adopt it after the dependency bump — the deeper duplication is that internal/ingest/contractevents is a near-clone of the SDK's processors/token_transfer V4 parsing, and this same one-line rule had to be applied to both.

The SDK PR also carries the ingest/sac decoders that back asset_stats_processor.go and contract_asset_stats.go; those reach Horizon transparently on the next SDK bump, with one behavior note about contract asset stats documented there.

Testing

internal/ingest/contractevents and internal/ingest/processors pass; gofmt and go vet clean. The DB-backed tests in internal/ingest were not run locally (no Postgres available).

🤖 Generated with Claude Code

tamirms and others added 2 commits August 19, 2026 10:21
parseSacEventMap required the data map to hold exactly two entries and to carry
a to_muxed_id. A transfer with no muxed destination may express that by
omitting the field, as a CAP-86 sparse map (protocol 28) would, or by binding
the key to Void, and neither shape parsed.

Require only amount, accept both encodings of a None to_muxed_id, and ignore
unrecognized keys. This is forward-compat hardening rather than a fix for
anything in ledger history: the Stellar Asset Contract emits a map only when a
muxed destination exists, and always with both keys. It matters because every
caller discards an event that fails to parse instead of surfacing the error, so
a shape this parser rejects silently costs that event its account_credited and
account_debited effects, its balance_changes operation details, and its
participants.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@tamirms

tamirms commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Closing as unnecessary.

The premise behind this change was that CAP-0086 sparse maps could reach SAC event data. They cannot, on two independent grounds:

  1. The SAC is not built with soroban-sdk. storage_types.rs and public_types.rs in rs-soroban-env use soroban_builtin_sdk_macros::contracttype — a separate macro crate compiled into the host — so the soroban-sdk v28 change that adopts the sparse map host functions does not reach it. Its event data map is built by get_amount_data_maybe_muxed with map_new plus explicit puts, never through the linear-memory host functions CAP-0086 touches, and it emits a map only when a muxed destination exists — always carrying both amount and to_muxed_id.
  2. Even if it were, there would be nothing to omit. Sparse encoding drops only fields whose value is Void, and every SAC storage type is Option-free.

So no event in ledger history hits the shapes this PR newly accepts, and none ever will without a separate CAP that changes ledger meta and forces a coordinated Horizon update anyway.

Two things surfaced here that may deserve their own issues, independent of CAP-0086:

  • Parse failures are invisible. Every caller discards an event that fails to parse instead of surfacing the error (effects_processor.go:1547, operations_processor.go:793, :1096). A shape this parser rejects silently costs that event its account_credited/account_debited effects, its balance_changes operation details, and its participants — no error, no metric. CAP-0067 already changed this event format once by adding to_muxed_id; if the format grows again before Horizon is updated, the loss is silent.
  • parseSacEventMap is a near-clone of the SDK's processors/token_transfer V4 parsing, diverging only in value policy and return type.

The companion SDK PR (stellar/go-stellar-sdk#5983) was reduced for the same reason and now carries only a genuine bug fix in processors/token_transfer, which decodes event data from arbitrary SEP-41 contracts and so is genuinely exposed to both encodings.

@tamirms tamirms closed this Aug 19, 2026
@tamirms
tamirms deleted the cap-86-sparse-udt-maps branch August 19, 2026 08:52
@tamirms
tamirms restored the cap-86-sparse-udt-maps branch August 19, 2026 09:03
@tamirms tamirms reopened this Aug 19, 2026
@tamirms

tamirms commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Reopened. This PR stands on the premise that CAP-0086 applies to contract storage as well as events, in which case a V4 SAC event data map can arrive sparsely encoded and this parser's exact-arity gate plus mandatory to_muxed_id would reject it — silently, since every caller discards an event that fails to parse rather than surfacing the error.

Disregard the previous comment.

@tamirms tamirms closed this Aug 19, 2026
@tamirms
tamirms deleted the cap-86-sparse-udt-maps branch August 19, 2026 09:05
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.

1 participant