Conversation
Tests for Scan, Value, Roundtrip, and String methods following the same pattern as AddressBytea tests.
All test files now use valid hex strings that can be converted to 32-byte BYTEA values instead of simple strings like "tx1", "tx2".
Part of the plan to store XDR data as raw bytes for efficiency.
Similar pattern to HashBytea but uses base64 encoding (standard for XDR) and supports variable-length data.
Changed OperationXDR field from string to XDRBytea for automatic base64/BYTEA conversion.
Cast the base64 XDR string to XDRBytea for proper database storage.
- Change UNNEST cast from text[] to bytea[] - Convert XDRBytea to raw bytes in BatchInsert - Update BatchCopy to pass raw bytes instead of pgtype.Text
Forces GraphQL to use a resolver for operationXdr to convert XDRBytea to base64 string for API consumers.
- Update test_utils.go to use types.XDRBytea - Regenerate GraphQL code with OperationXdr resolver
Returns the XDRBytea as a base64-encoded string for API consumers.
- Use parameterized queries with XDRBytea for SQL inserts - Update assertions to use .String() for comparison - Fix type casting in test data creation
- Use base64-encoded test XDR data in test_utils.go - Add testOpXDR helper functions in test files - Update all assertions to use .String() method
- Update createTestOperation to use proper base64 XDR - Update generateTestOperations to encode test data as base64 - Update operations_test.go with base64 encoding for all test XDR data - Fix assertions to compare against the stored XDRBytea values
This simplifies the XDR storage flow by storing raw bytes directly instead of encoding to base64 and then decoding. The String() method now handles base64 encoding for external representation.
Skip the intermediate base64 encoding step by using MarshalBinary() instead of MarshalBase64(). The raw bytes are now stored directly in XDRBytea.
Remove unnecessary Value() calls since XDRBytea is now []byte. Access raw bytes directly via type conversion.
Decode expected base64 XDR string to raw bytes for comparison since XDRBytea now uses []byte underlying type.
Use raw bytes directly instead of base64-encoded strings when creating test data for XDRBytea fields.
Use raw bytes directly for test XDR data instead of base64-encoding. The String() method will handle base64 encoding for assertions.
Use raw bytes directly instead of pre-encoded base64 string.
Use parameterized queries instead of raw SQL string literals for BYTEA operation_xdr column. Fix .String() assertion to compare base64 values via opXdr1.String().
Use parameterized queries instead of raw SQL string literals for BYTEA operation_xdr column in BatchGetByOperationIDs and BatchGetByStateChangeIDs tests.
Use parameterized queries instead of raw SQL string literals for BYTEA operation_xdr column in BatchGetByOperationIDs test.
There was a problem hiding this comment.
Pull request overview
This PR migrates the codebase’s PostgreSQL access layer from lib/pq + sqlx patterns to pgx/v5 (pool + tx), updating models, services, and tests accordingly.
Changes:
- Replaces
db.ConnectionPool/sqlxusage with*pgxpool.Pool, and introducesdb.Querier+ genericdb.QueryOne/QueryManyhelpers. - Updates transaction handling to use a single
db.RunInTransactionhelper (pgx tx), and ports query parameterization to pgx-compatible forms. - Removes DB-bound metrics initialization (drops sqlstats collector wiring) and updates callers/tests to the new metrics constructor.
Reviewed changes
Copilot reviewed 76 out of 77 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/utils/db_errors_test.go | Updates DB error typing tests to use pgx/pgconn error types. |
| internal/utils/db_errors.go | Switches pq error classification to pgx/pgconn + pgx.ErrNoRows handling. |
| internal/signing/store/types.go | Updates store interfaces to pgx pool/tx types. |
| internal/signing/store/mocks.go | Updates mocks to match revised store interfaces. |
| internal/signing/store/keypairs_model_test.go | Ports keypair fixtures/tests to pgxpool + positional args. |
| internal/signing/store/keypairs_model.go | Migrates keypair model to pgxpool + pgconn errors + QueryOne. |
| internal/signing/store/channel_accounts_model_test.go | Ports channel account tests to pgxpool/pgx tx and array args. |
| internal/signing/store/channel_accounts_model.go | Migrates channel account model to pgxpool + QueryOne/QueryMany + pgx tx methods. |
| internal/signing/kms_signature_client_test.go | Updates DB pool creation to pass context and pgxpool type. |
| internal/signing/channel_account_db_signature_client_test.go | Updates mock expectations for revised store method signatures. |
| internal/signing/channel_account_db_signature_client.go | Switches signature client DB dependency to *pgxpool.Pool and updated store calls. |
| internal/services/transaction_service_test.go | Updates DB pool construction to new signature (ctx + pgxpool). |
| internal/services/transaction_service.go | Switches TransactionServiceOptions DB type to *pgxpool.Pool. |
| internal/services/token_ingestion_test.go | Updates transaction helper name and pgxpool Exec usage in tests. |
| internal/services/token_ingestion.go | Switches service DB dependency to *pgxpool.Pool and new transaction helper. |
| internal/services/rpc_service_test.go | Updates DB pool initialization to new signature. |
| internal/services/kms_import_key_service_test.go | Updates DB pool initialization to new signature. |
| internal/services/ingest_live.go | Updates ingestion transaction helper call. |
| internal/services/ingest_backfill.go | Updates recompressor + transaction helper usage for pgxpool. |
| internal/services/fee_bump_service_test.go | Ports ExecContext to Exec and updates pool initialization. |
| internal/services/channel_account_service_test.go | Updates mocks and pool initialization to new signatures. |
| internal/services/channel_account_service.go | Switches service DB dependency to *pgxpool.Pool and pgx tx transaction helper. |
| internal/serve/serve.go | Removes sqlx-based metrics wiring; uses new metrics constructor; passes ctx to DB open. |
| internal/serve/httphandler/health_test.go | Updates ingest_store inserts to string values and pgxpool Exec calls. |
| internal/serve/graphql/resolvers/utils.go | Updates state change cursor field reference after cursor struct change. |
| internal/serve/graphql/resolvers/transaction.resolvers.go | Updates cursor access to embedded CompositeCursor. |
| internal/serve/graphql/resolvers/test_utils.go | Ports resolver test DB setup/cleanup to pgxpool and pgx tx. |
| internal/serve/graphql/resolvers/setup_test.go | Switches test DB pool global to *pgxpool.Pool and new open signature. |
| internal/serve/graphql/resolvers/queries.resolvers.go | Updates composite cursor formatting after cursor embedding. |
| internal/serve/graphql/resolvers/account.resolvers.go | Updates composite cursor formatting after cursor embedding. |
| internal/serve/graphql/dataloaders/statechange_loaders.go | Updates key functions to reflect embedded state change/cursor shapes. |
| internal/serve/graphql/dataloaders/operation_loaders.go | Updates operation ID access after embedding changes. |
| internal/metrics/metrics_test.go | Removes sqlite/sqlx setup and updates metrics service constructor usage. |
| internal/metrics/metrics.go | Removes sqlstats/sqlx dependencies and DB collector registration. |
| internal/loadtest/runner.go | Removes sqlx metrics dependency and updates tx helper usage. |
| internal/integrationtests/infrastructure/main_setup.go | Ports DB reads and metrics service init away from sqlx; updates pool opening signature. |
| internal/integrationtests/infrastructure/backfill_helpers.go | Replaces database/sql direct usage with pgxpool-based helpers. |
| internal/ingest/timescaledb_test.go | Ports tests to pgxpool + QueryOne helpers. |
| internal/ingest/timescaledb.go | Updates hypertable configuration helper to accept *pgxpool.Pool and pgx query APIs. |
| internal/ingest/ingest.go | Switches pool open calls to ctx-based pgxpool versions; removes sqlx metrics wiring. |
| internal/indexer/types/types.go | Embeds cursor structs directly in “WithCursor” types (CompositeCursor/StateChangeCursor). |
| internal/db/utils_test.go | Updates advisory lock tests to use pgxpool + pinned connections. |
| internal/db/utils.go | Updates advisory lock helpers to accept a Querier and use pgx QueryRow/Scan. |
| internal/db/migrate_test.go | Ports migration tests to pgxpool + QueryOne/QueryMany helpers. |
| internal/db/migrate.go | Switches migration execution to use stdlib sql.DB wrapper from pgx pool. |
| internal/db/db_test.go | Updates DB open test to new pgxpool signature. |
| internal/db/db.go | Replaces ConnectionPool/sqlx abstractions with pgxpool, Querier, tx helper, and query helpers. |
| internal/data/trustline_balances_test.go | Ports trustline balance tests to pgxpool Exec and new open signature. |
| internal/data/trustline_balances.go | Migrates trustline balance querying to QueryMany with struct db tags. |
| internal/data/trustline_assets_test.go | Ports helper queries and tx begins to pgxpool/pgx. |
| internal/data/trustline_assets.go | Switches model DB dependency to *pgxpool.Pool. |
| internal/data/transactions_test.go | Ports transactions tests and benchmark metrics wiring to new metrics constructor + pgxpool open. |
| internal/data/transactions.go | Migrates transaction queries to QueryOne/QueryMany and pgx-compatible cursor aliases. |
| internal/data/statechanges.go | Migrates state changes queries to QueryMany and pgx-compatible cursor aliases. |
| internal/data/sac_balances_test.go | Ports SAC balance tests to pgxpool Exec and tx begin. |
| internal/data/sac_balances.go | Migrates SAC balance querying to QueryMany with proper column aliases/tags. |
| internal/data/operations.go | Migrates operation queries to QueryOne/QueryMany and pgx-compatible cursor aliases. |
| internal/data/native_balances_test.go | Ports native balance tests to pgxpool Exec and tx begin. |
| internal/data/native_balances.go | Migrates native balance querying to QueryOne with struct db tags. |
| internal/data/models.go | Switches Models.DB to *pgxpool.Pool and updates model constructors. |
| internal/data/ingest_store_test.go | Ports ingest_store tests to pgxpool and string value inserts; updates tx helper name. |
| internal/data/ingest_store.go | Migrates ingest_store reads to QueryOne(string) + parsing; ports list queries to QueryMany. |
| internal/data/contract_tokens_test.go | Ports contract model tests to new tx helper and QueryOne reads. |
| internal/data/contract_tokens.go | Migrates GetExisting to QueryMany (scalar) and updates DB type. |
| internal/data/accounts_test.go | Ports accounts tests to pgxpool Exec and new open signature. |
| internal/data/accounts.go | Migrates account queries to QueryOne/QueryMany and updates DB type. |
| internal/data/account_contract_tokens.go | Migrates contract list query to QueryMany and updates DB type. |
| go.mod | Removes direct deps on pq/sqlx/sqlite/sqlstats and reclassifies some as indirect. |
| cmd/utils/utils.go | Updates signature client options DB type to *pgxpool.Pool. |
| cmd/serve.go | Removes pq driver import; updates DB pool opening to ctx-based signature. |
| cmd/ingest.go | Removes pq driver import. |
| cmd/distribution_account.go | Updates DB pool opening to ctx-based signature. |
| cmd/channel_account.go | Removes sqlx-based metrics wiring; updates pool close behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pq code to pgxpq code to pgx
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 79 out of 80 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
internal/db/utils.go:28
ReleaseAdvisoryLockscans the boolean result ofpg_advisory_unlockintoreleasedbut then ignores it and always returns nil. This makes it impossible to detect when the lock was not actually held/released. Consider returning an error (or returning the boolean) whenreleasedis false so callers can react appropriately.
internal/metrics/metrics.go:377NewMetricsService/registerMetricsno longer register any DB/pool-level metrics (previously done viasqlstats). This is a functional observability regression: operators will lose visibility into DB connection usage/health via Prometheus. Consider adding an equivalent collector based onpgxpool.Pool.Stat()(or wiring a*sql.DBviastdlib.OpenDBFromPoolinto a collector) so existing DB metrics remain available after thepgxmigration.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| )) | ||
|
|
||
| m.registry.MustRegister(prometheus.NewGaugeFunc( | ||
| prometheus.GaugeOpts{ |
There was a problem hiding this comment.
should this use a CounterFunc instead? Most other cumulative metrics do.
| // checking. This cannot be set via connection string and requires elevated privileges (superuser | ||
| // or replication role). Call this ONCE at backfill startup after creating the connection pool. | ||
| func ConfigureBackfillSession(ctx context.Context, db ConnectionPool) error { | ||
| _, err := db.ExecContext(ctx, "SET session_replication_role = 'replica'") |
There was a problem hiding this comment.
it looks like we didn't port this over, do we no longer need it?
| MaxIdleDBConns = 20 // Keep warm connections ready in the pool | ||
| MaxDBConnLifetime = 5 * time.Minute // Recycle connections periodically | ||
| DefaultMaxConnIdleTime time.Duration = 10 * time.Second | ||
| DefaultMaxConns int32 = 10 |
There was a problem hiding this comment.
This change looks unrelated to the library migration. Is this informed by our dev instance metrics?
Bug:
|
Bug:
|
What
lib/pqto usingpgxWhy
lib/pqlibrary is not actively developed and is now under maintenance only. Thepgxlibrary is more active in resolving bugs and adding new features. We were already utilizingpgxforCOPYinserts since it supported fast binary protocol copy inserts. However, it does not make sense to support 2 different database libraries in the same code, given that we are going to actively add new features to wallet backend.This cleanup ensures we consistently use the same
pgxlibrary everywhere in the codeKnown limitations
N/A
Issue that this PR addresses
Closes #529