From 75c32cb2dafbfaf9d354dcfdce001affdb33db88 Mon Sep 17 00:00:00 2001 From: brite-side0 Date: Sat, 25 Jul 2026 08:16:15 +0100 Subject: [PATCH 1/2] feat(db): enforce natural-key uniqueness on soroban_events (tx_hash, event_index, network) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add migration 0010 that adds UNIQUE (transaction_hash, event_index, network) constraint to soroban_events. Includes a pre-flight duplicate check that raises an exception if any conflicting rows exist before applying the DDL. - Decision record (in migration + db/mod.rs docs): The UUIDv5 id is derived from (contract_id, ledger_sequence, event_index) and is NOT a pure function of the Stellar protocol natural key (transaction_hash, event_index). The new constraint is therefore a complementary, independent correctness guard that enforces the protocol- level uniqueness guarantee regardless of the id derivation scheme. The constraint is network-scoped to allow the same tx hash to appear on different networks (testnet / mainnet) in multi-network deployments and to align with the partition-key candidate identified in #244. - Update schema.sql to mirror migration 0010 end state (bump comment from 0001-0009 to 0001-0010, add constraint). - Propagate network parameter through both insert paths: crates/indexer/src/db/mod.rs — insert_event now takes &str network; populates the network column explicitly instead of relying on the DEFAULT 'testnet' column default. crates/backfill/src/db.rs — same signature change. crates/indexer/src/streamer/mod.rs — passes self.config.network. crates/backfill/src/main.rs — clones args.network into each worker. - Add two new integration tests in crates/indexer/src/db/mod.rs: event_uuid_does_not_include_transaction_hash — documents that the UUID key is distinct from the natural key. natural_key_constraint_rejects_duplicate_tx_event_index — inserts two events with different contract_ids but the same (tx_hash, event_index, network) and asserts the second insert is rejected by the constraint. Closes #. Targets dev. --- crates/backfill/src/db.rs | 20 ++- crates/backfill/src/main.rs | 3 +- crates/indexer/src/db/mod.rs | 156 ++++++++++++++++-- crates/indexer/src/streamer/mod.rs | 2 +- .../0010_soroban_events_natural_key.sql | 55 ++++++ database/schema.sql | 12 +- 6 files changed, 231 insertions(+), 17 deletions(-) create mode 100644 database/migrations/0010_soroban_events_natural_key.sql diff --git a/crates/backfill/src/db.rs b/crates/backfill/src/db.rs index 2cb3aa3..a7eed44 100644 --- a/crates/backfill/src/db.rs +++ b/crates/backfill/src/db.rs @@ -4,12 +4,25 @@ use uuid::Uuid; const EVENT_NS: Uuid = Uuid::NAMESPACE_DNS; +/// Derive a deterministic UUID for an event from its indexer-internal composite key. +/// +/// The UUID is derived from `(contract_id, ledger_sequence, event_index)`. +/// See `crates/indexer/src/db/mod.rs` for the full rationale and the relationship +/// to the natural key `(transaction_hash, event_index, network)`. fn event_uuid(contract_id: &str, ledger_sequence: u64, event_index: u32) -> Uuid { let key = format!("{contract_id}:{ledger_sequence}:{event_index}"); Uuid::new_v5(&EVENT_NS, key.as_bytes()) } -pub async fn insert_event(pool: &PgPool, event: &SorobanEvent) -> Result<(), TridentError> { +/// Insert a backfilled event. Duplicate handling is identical to the indexer: +/// - `ON CONFLICT (id) DO NOTHING` deduplicates replays. +/// - The DB-level `UNIQUE (transaction_hash, event_index, network)` constraint +/// (migration 0010) provides an independent protocol-level guard. +pub async fn insert_event( + pool: &PgPool, + event: &SorobanEvent, + network: &str, +) -> Result<(), TridentError> { let id = event_uuid(&event.contract_id, event.ledger_sequence, event.event_index); let event_type = match event.event_type { trident_common::EventType::Contract => "contract", @@ -26,8 +39,8 @@ pub async fn insert_event(pool: &PgPool, event: &SorobanEvent) -> Result<(), Tri r#" INSERT INTO soroban_events (id, contract_id, ledger_sequence, ledger_timestamp, transaction_hash, - event_index, event_type, topics, data) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) + event_index, event_type, topics, data, network) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) ON CONFLICT (id) DO NOTHING "#, ) @@ -40,6 +53,7 @@ pub async fn insert_event(pool: &PgPool, event: &SorobanEvent) -> Result<(), Tri .bind(event_type) .bind(&topics) .bind(&event.data) + .bind(network) .execute(pool) .await .map_err(|e| TridentError::storage(anyhow::Error::new(e).context("insert_event")))?; diff --git a/crates/backfill/src/main.rs b/crates/backfill/src/main.rs index 3bf6c0a..1e680a3 100644 --- a/crates/backfill/src/main.rs +++ b/crates/backfill/src/main.rs @@ -98,6 +98,7 @@ async fn main() -> Result<(), Box> { let duplicates_skipped = duplicates_skipped.clone(); let pb = pb.clone(); let rpc_delay = args.rpc_delay_ms; + let network = args.network.clone(); let handle = tokio::spawn(async move { while let Some((s, e)) = rx.lock().await.recv().await { @@ -125,7 +126,7 @@ async fn main() -> Result<(), Box> { println!("DRY: event {:?}", ev); } } else { - match db::insert_event(&db, &ev).await { + match db::insert_event(&db, &ev, &network).await { Ok(_) => { events_indexed.fetch_add(1, Ordering::Relaxed); } diff --git a/crates/indexer/src/db/mod.rs b/crates/indexer/src/db/mod.rs index 141ed2a..1eb5d5e 100644 --- a/crates/indexer/src/db/mod.rs +++ b/crates/indexer/src/db/mod.rs @@ -32,18 +32,46 @@ pub async fn connect_pool(database_url: &str, pool_size: u32) -> Result Uuid { let key = format!("{contract_id}:{ledger_sequence}:{event_index}"); Uuid::new_v5(&EVENT_NS, key.as_bytes()) } -/// Insert a normalised event. Silently ignores duplicates via `ON CONFLICT (id) DO NOTHING`. -/// The `id` is a deterministic UUIDv5 derived from `(contract_id, ledger_sequence, event_index)`, -/// so replaying the same event always produces the same primary key. -pub async fn insert_event(pool: &PgPool, event: &SorobanEvent) -> Result<(), TridentError> { +/// Insert a normalised event. +/// +/// Duplicate handling uses two complementary strategies: +/// - **Primary**: `ON CONFLICT (id) DO NOTHING` — deduplicates replays because `id` +/// is a deterministic UUIDv5 derived from `(contract_id, ledger_sequence, event_index)`. +/// - **Safety net**: `UNIQUE (transaction_hash, event_index, network)` at the DB layer +/// (migration 0010) catches any case where the same protocol event would be inserted +/// with a different derived `id` (e.g. due to a bug in id derivation). +/// +/// The `network` argument must match the value used in `indexed_contracts` for this +/// deployment (e.g. `"mainnet"` or `"testnet"`). +pub async fn insert_event( + pool: &PgPool, + event: &SorobanEvent, + network: &str, +) -> Result<(), TridentError> { let id = event_uuid(&event.contract_id, event.ledger_sequence, event.event_index); let event_type = match event.event_type { EventType::Contract => "contract", @@ -60,8 +88,8 @@ pub async fn insert_event(pool: &PgPool, event: &SorobanEvent) -> Result<(), Tri r#" INSERT INTO soroban_events (id, contract_id, ledger_sequence, ledger_timestamp, transaction_hash, - event_index, event_type, topics, data) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) + event_index, event_type, topics, data, network) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) ON CONFLICT (id) DO NOTHING "#, ) @@ -74,6 +102,7 @@ pub async fn insert_event(pool: &PgPool, event: &SorobanEvent) -> Result<(), Tri .bind(event_type) .bind(&topics) .bind(&event.data) + .bind(network) .execute(pool) .await .map_err(|e| TridentError::storage(anyhow::Error::new(e).context("insert_event")))?; @@ -293,6 +322,29 @@ mod tests { assert_ne!(a, b); } + /// The UUID id is NOT derived from transaction_hash, so two events with + /// different contract_ids but the same (transaction_hash, event_index) would + /// produce different UUIDs. This test documents that distinction — the + /// natural-key constraint (uq_soroban_events_tx_index_network) is the guard + /// for that case. + #[test] + fn event_uuid_does_not_include_transaction_hash() { + // Same (contract_id, ledger, event_index) → same UUID regardless of tx_hash. + let uuid_a = event_uuid("CABC", 100, 0); + let uuid_b = event_uuid("CABC", 100, 0); // identical inputs, different tx_hash in the event struct + assert_eq!( + uuid_a, uuid_b, + "UUID must be stable across calls with the same indexer key" + ); + + // Different contract_id → different UUID even if tx+index were the same. + let uuid_c = event_uuid("CXYZ", 100, 0); + assert_ne!( + uuid_a, uuid_c, + "different contract_id must produce a different UUID" + ); + } + /// Calling `insert_event` twice with the same event must not error and /// the row count in `soroban_events` must remain 1. /// @@ -323,10 +375,10 @@ mod tests { .await .expect("cleanup failed"); - insert_event(&pool, &event) + insert_event(&pool, &event, "testnet") .await .expect("first insert failed"); - insert_event(&pool, &event) + insert_event(&pool, &event, "testnet") .await .expect("second insert must not error"); @@ -339,4 +391,86 @@ mod tests { assert_eq!(count.0, 1, "duplicate insert should be silently ignored"); } + + /// Inserting two events with the same (transaction_hash, event_index, network) but + /// different contract_ids (which would produce different UUIDs) must be rejected + /// by the natural-key constraint `uq_soroban_events_tx_index_network`. + /// + /// This validates that the DB-level guard works independently of the id scheme. + #[tokio::test] + async fn natural_key_constraint_rejects_duplicate_tx_event_index() { + let db_url = match std::env::var("TEST_DATABASE_URL") { + Ok(url) => url, + Err(_) if std::env::var("REQUIRE_TEST_SERVICES").is_ok() => { + panic!("TEST_DATABASE_URL must be set when REQUIRE_TEST_SERVICES is set"); + } + Err(_) => { + eprintln!("SKIP: TEST_DATABASE_URL not set"); + return; + } + }; + let pool = PgPool::connect(&db_url).await.unwrap(); + + // Shared (transaction_hash, event_index) — this is the natural key. + let shared_tx_hash = "txhash_natural_key_test_001"; + let shared_event_index: u32 = 0; + let network = "testnet"; + + // Clean up any leftovers from previous runs. + sqlx::query("DELETE FROM soroban_events WHERE transaction_hash = $1") + .bind(shared_tx_hash) + .execute(&pool) + .await + .expect("cleanup failed"); + + // First event: contract A, same tx+index. + let event_a = SorobanEvent { + contract_id: "CONTRACT_A_NATURAL_KEY_TEST".to_string(), + ledger_sequence: 999, + ledger_timestamp: "2024-01-01T00:00:00Z".to_string(), + transaction_hash: shared_tx_hash.to_string(), + event_index: shared_event_index, + event_type: EventType::Contract, + topics: vec![], + data: json!({}), + }; + insert_event(&pool, &event_a, network) + .await + .expect("first insert (contract A) must succeed"); + + // Second event: DIFFERENT contract_id → DIFFERENT UUID, but SAME (tx_hash, event_index, network). + // The natural-key constraint must reject this. + let event_b = SorobanEvent { + contract_id: "CONTRACT_B_NATURAL_KEY_TEST".to_string(), + ledger_sequence: 999, + ledger_timestamp: "2024-01-01T00:00:00Z".to_string(), + transaction_hash: shared_tx_hash.to_string(), + event_index: shared_event_index, + event_type: EventType::Contract, + topics: vec![], + data: json!({}), + }; + let result = insert_event(&pool, &event_b, network).await; + assert!( + result.is_err(), + "inserting a duplicate (transaction_hash, event_index, network) with a different \ + contract_id must be rejected by uq_soroban_events_tx_index_network" + ); + + // Verify exactly one row persisted. + let count: (i64,) = + sqlx::query_as("SELECT COUNT(*) FROM soroban_events WHERE transaction_hash = $1") + .bind(shared_tx_hash) + .fetch_one(&pool) + .await + .expect("count query failed"); + assert_eq!(count.0, 1, "only the first event should be stored"); + + // Cleanup. + sqlx::query("DELETE FROM soroban_events WHERE transaction_hash = $1") + .bind(shared_tx_hash) + .execute(&pool) + .await + .expect("post-test cleanup failed"); + } } diff --git a/crates/indexer/src/streamer/mod.rs b/crates/indexer/src/streamer/mod.rs index 82fd191..c1481b4 100644 --- a/crates/indexer/src/streamer/mod.rs +++ b/crates/indexer/src/streamer/mod.rs @@ -267,7 +267,7 @@ impl Streamer { continue; } } - db::insert_event(&self.db, &event) + db::insert_event(&self.db, &event, &self.config.network) .instrument(tracing::info_span!( "db_insert_events", contract_id = %event.contract_id diff --git a/database/migrations/0010_soroban_events_natural_key.sql b/database/migrations/0010_soroban_events_natural_key.sql new file mode 100644 index 0000000..90fe057 --- /dev/null +++ b/database/migrations/0010_soroban_events_natural_key.sql @@ -0,0 +1,55 @@ +-- Migration 0010: enforce natural-key uniqueness on soroban_events +-- --------------------------------------------------------------------------- +-- Decision record +-- --------------- +-- The `id` column is a deterministic UUIDv5 derived from +-- (contract_id, ledger_sequence, event_index) — it is NOT a pure function of +-- the Stellar protocol's natural key (transaction_hash, event_index). +-- Therefore a separate unique constraint on the natural key is needed to: +-- 1. Guard against genuine duplicates that would produce different UUIDs +-- (e.g. same tx/index pair arriving via two different code paths with +-- different contract_id or ledger_sequence values due to a bug). +-- 2. Provide a constraint target that mirrors the Stellar protocol guarantee: +-- within a given network a (transaction_hash, event_index) pair +-- identifies exactly one event. +-- 3. Future-proof the schema against id-scheme changes without losing +-- the correctness guarantee. +-- +-- The constraint is network-scoped because the same transaction hash CAN +-- appear on different networks in test/local environments, and because +-- the `network` column is the partition key candidate per issue #244. +-- +-- The existing `ON CONFLICT (id) DO NOTHING` insert strategy is preserved +-- as-is; the natural-key constraint is an additional safety net at the DB +-- layer that catches any bugs in id derivation. +-- --------------------------------------------------------------------------- + +-- Validate existing data before adding the constraint. +-- If any duplicates exist they will surface here and must be resolved before +-- deploying this migration. In a fresh database (CI, new deployments) this +-- is a no-op. +DO $$ +DECLARE + dup_count BIGINT; +BEGIN + SELECT COUNT(*) + INTO dup_count + FROM ( + SELECT transaction_hash, event_index, network + FROM soroban_events + GROUP BY transaction_hash, event_index, network + HAVING COUNT(*) > 1 + ) sub; + + IF dup_count > 0 THEN + RAISE EXCEPTION + 'Cannot add natural-key constraint: % duplicate (transaction_hash, event_index, network) group(s) found. ' + 'Resolve duplicates before applying this migration.', + dup_count; + END IF; +END $$; + +-- Add the unique constraint. +ALTER TABLE soroban_events + ADD CONSTRAINT uq_soroban_events_tx_index_network + UNIQUE (transaction_hash, event_index, network); diff --git a/database/schema.sql b/database/schema.sql index ad7c52e..9b48da8 100644 --- a/database/schema.sql +++ b/database/schema.sql @@ -1,6 +1,6 @@ -- Trident PostgreSQL Schema -- Convenience full-schema snapshot for local/dev bootstrap and documentation. --- The migration chain in ./migrations/ (0001-0009) is the source of truth and is +-- The migration chain in ./migrations/ (0001-0010) is the source of truth and is -- what CI and production apply; this file must mirror the end state of that chain. -- Keep in sync whenever a migration is added. @@ -37,6 +37,16 @@ CREATE INDEX IF NOT EXISTS idx_soroban_events_contract_topic0 ON soroban_events CREATE INDEX IF NOT EXISTS idx_soroban_events_id_desc ON soroban_events (id DESC); CREATE INDEX IF NOT EXISTS idx_soroban_events_ledger_timestamp ON soroban_events (ledger_timestamp DESC); +-- Natural-key uniqueness (0010). +-- The `id` UUIDv5 is derived from (contract_id, ledger_sequence, event_index) and +-- is NOT a pure function of the Stellar protocol natural key (transaction_hash, +-- event_index). This constraint enforces the protocol-level guarantee independently +-- of the id scheme and is network-scoped to allow the same tx hash to exist on +-- different networks (test/mainnet). See database/migrations/0010 for full rationale. +ALTER TABLE soroban_events + ADD CONSTRAINT uq_soroban_events_tx_index_network + UNIQUE (transaction_hash, event_index, network); + -- --------------------------------------------------------------------------- -- system_state -- Persistent cursor tracking so the indexer can resume after restart without From 0c55517bd5ab89cae437b5cf4c7536a0c8591b14 Mon Sep 17 00:00:00 2001 From: Depo-dev Date: Fri, 31 Jul 2026 17:09:28 +0100 Subject: [PATCH 2/2] fix(db): include the partition key in the natural-key constraint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Rust job failed at "Run migrations". 0025 tried ALTER TABLE soroban_events ADD CONSTRAINT uq_soroban_events_tx_index_network UNIQUE (transaction_hash, event_index, network); but migration 0017 — which landed on dev after this branch was opened — made soroban_events RANGE-partitioned by ledger_sequence, and PostgreSQL requires every unique constraint on a partitioned table to include the partition key. The statement is rejected outright, so no fresh database can apply the chain. Added ledger_sequence to the constraint. This is the same trade-off 0017 already made and documented for the primary key, which became (ledger_sequence, id) for exactly this reason. The cost is recorded in the migration rather than glossed over: the constraint no longer catches the same (transaction_hash, event_index, network) triple appearing under two different ledger_sequence values. That case would mean the indexer attributed one protocol event to two ledgers — a different bug from the duplicate-insert this migration targets, and one the pre-flight duplicate check above still catches on existing data. Within a ledger, which is where replays and overlapping code paths actually produce duplicates, the guarantee is unchanged. --- .../0025_soroban_events_natural_key.sql | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/database/migrations/0025_soroban_events_natural_key.sql b/database/migrations/0025_soroban_events_natural_key.sql index 634abd4..0f708d7 100644 --- a/database/migrations/0025_soroban_events_natural_key.sql +++ b/database/migrations/0025_soroban_events_natural_key.sql @@ -11,7 +11,8 @@ -- different contract_id or ledger_sequence values due to a bug). -- 2. Provide a constraint target that mirrors the Stellar protocol guarantee: -- within a given network a (transaction_hash, event_index) pair --- identifies exactly one event. +-- identifies exactly one event. (Scoped per-partition in practice — see +-- the note on ledger_sequence at the ALTER TABLE below.) -- 3. Future-proof the schema against id-scheme changes without losing -- the correctness guarantee. -- @@ -50,6 +51,22 @@ BEGIN END $$; -- Add the unique constraint. +-- +-- ledger_sequence is part of the key only because it has to be: migration +-- 0017 made soroban_events RANGE-partitioned on that column, and PostgreSQL +-- requires every unique constraint on a partitioned table to include the +-- partition key. `UNIQUE (transaction_hash, event_index, network)` alone is +-- rejected outright — the same trade-off 0017 already documents for the +-- primary key, which became (ledger_sequence, id) for this reason. +-- +-- What this costs: the constraint no longer catches the same +-- (transaction_hash, event_index, network) triple appearing under two +-- *different* ledger_sequence values. That would mean the indexer recorded +-- one protocol event against two different ledgers, which is a distinct bug +-- from the duplicate-insert case this migration exists to catch, and it is +-- still caught by the pre-flight check above on any existing data. Within a +-- ledger — where duplicates actually arise, from replays and overlapping +-- code paths — the guarantee is unchanged. ALTER TABLE soroban_events ADD CONSTRAINT uq_soroban_events_tx_index_network - UNIQUE (transaction_hash, event_index, network); + UNIQUE (ledger_sequence, transaction_hash, event_index, network);