RPC method getSignaturesForAddress implemented#95
Conversation
Greptile SummaryThis PR implements the
Confidence Score: 4/5Safe to merge for Postgres-only deployments; the Redis path has a P1 issue that silently defeats the idempotency guarantee mint.rs relies on. The Postgres implementation is clean, well-tested, and correctly atomic. The P1 finding (Redis returning Ok(vec![]) instead of Err) is a real correctness defect on the Redis code path: it directly contradicts the PR's stated design rationale and could cause duplicate mints if Redis is the active backend. The P2 findings on silent cursor-field dropping and non-deterministic same-slot order are non-blocking for the current use case. core/src/accounts/get_signatures_for_address.rs (Redis path), core/src/rpc/get_signatures_for_address_impl.rs (cursor fields)
|
| Filename | Overview |
|---|---|
| core/src/accounts/get_signatures_for_address.rs | New file implementing the DB query layer; Postgres path is solid (single JOIN, proper error propagation), but the Redis path silently returns Ok(vec![]) despite no indexing, breaking the idempotency guarantee that is the PR's stated design rationale. |
| core/src/accounts/write_batch.rs | Indexes every account key into address_signatures within the existing atomic Postgres transaction; ON CONFLICT DO NOTHING is correct. Redis write_batch has no corresponding index, consistent with the silent empty-vec return issue. |
| core/src/accounts/postgres.rs | Adds the address_signatures table with composite PK (address, slot, signature) using IF NOT EXISTS; schema is correct and idempotent. |
| core/src/rpc/get_signatures_for_address_impl.rs | Parses address and clamps limit correctly; before/until cursor fields from the config are silently dropped with no error, which could mislead callers expecting pagination behavior. |
| gateway/src/auth.rs | Correctly adds getSignaturesForAddress to ACCOUNT_GATED_METHODS; auth gating follows the same pattern as getAccountInfo with operator bypass and user ownership check. |
| core/src/accounts/traits.rs | Adds get_signatures_for_address method to AccountsDB; signature and delegation are consistent with other methods. |
| gateway/tests/auth_integration.rs | Adds 4 integration tests covering 401 (no token), 200 (owned wallet), 403 (unowned), and 200 (operator); coverage matches the auth scenarios. |
| core/src/rpc/api.rs | Declares getSignaturesForAddress on the ContraRpc trait; signature matches the impl. |
Sequence Diagram
sequenceDiagram
participant Client
participant Gateway
participant RPC as Core RPC
participant DB as AccountsDB
Client->>Gateway: POST getSignaturesForAddress {address, params}
Gateway->>Gateway: JWT validation (ACCOUNT_GATED_METHODS)
alt No token
Gateway-->>Client: 401 Unauthorized
else User role
Gateway->>RPC: getAccountInfo(address) [Phase 2 ownership check]
RPC-->>Gateway: account data
alt Ownership check fails
Gateway-->>Client: 403 Forbidden
else Ownership OK
Gateway->>RPC: proxy getSignaturesForAddress
end
else Operator role
Gateway->>RPC: proxy getSignaturesForAddress (unrestricted)
end
RPC->>RPC: parse address, clamp limit [1,1000]
RPC->>DB: get_signatures_for_address(pubkey, limit)
alt Postgres backend
DB->>DB: SELECT sig, slot, tx.data FROM address_signatures LEFT JOIN transactions ORDER BY slot DESC LIMIT $2
DB-->>RPC: Vec of RpcConfirmedTransactionStatusWithSignature
else Redis backend
DB-->>RPC: Ok(vec![]) [unimplemented — silent empty]
end
RPC-->>Client: JSON-RPC result
Reviews (1): Last reviewed commit: "RPC method getSignaturesForAddress imple..." | Re-trigger Greptile
e9e9f54 to
4e6f78b
Compare
dev-jodee
left a comment
There was a problem hiding this comment.
two questions / discussions
| const ACCOUNT_GATED_METHODS: &[&str] = &[ | ||
| "getAccountInfo", | ||
| "getTokenAccountBalance", | ||
| "getSignaturesForAddress", |
There was a problem hiding this comment.
I think there's a corner case that's going to fail if we set on getSignaturesForAddress the current auth flow:
Here the check for account exists is before account owned by user
Thinking out loud here, but if some accounts get closed, like closes TokenAccount, then that first if would fail, and user couldn't get signatures for an account he used to have ? Might not be an issue but curious to see what you think
There was a problem hiding this comment.
Good catch, you're right. Once a TokenAccount is closed, check_account_data_ownership falls back to is_wallet_owned_by_user on the TA address directly, which always fails.
I see three options:
- Accept the limitation. No real incentive for users to close these accounts in our context (no rent to claim).
- Optional mint param. Derive the ATA and check if it matches (partial solution, ATAs only).
- Snapshot ownership at index time. Store (address -> owner_wallet) in write_batch.rs during ingest. Complete solution but requires schema changes and extra work at ingest.
I'm leaning toward 1 for now, but curious about your thoughts?
| let key = format!("addr_sigs:{}", address); | ||
|
|
||
| // ZRANGE ... BYSCORE REV returns members with the highest score (most recent | ||
| // slot) first, matching the Postgres ORDER BY slot DESC behaviour. |
There was a problem hiding this comment.
Is this going to have a different behavior than postgres since postgres has on slot and then on signature (https://github.com/solana-foundation/contra/pull/95/changes#diff-e1e9b8a701448e47decc131fc65967e2bc6d0b951829b2877bf49c2abc573736R138)
There was a problem hiding this comment.
Thanks, fixed by storing members as hex instead of base58, which preserves byte ordering and matches Postgres's ORDER BY signature DESC
…ch Postgres ordering, bump test Redis to 7.0
feat(rpc): implement
getSignaturesForAddressImplements the
getSignaturesForAddressSolana JSON-RPC method end-to-end.What changed
address_signaturestable: new table(address, slot, signature)indexed via composite PK. Written atomically withtransactionsso they're always consistent.get_signatures_for_address.rs: single LEFT JOIN query,slot DESCordering,before/untilcursor pagination for Postgres (Redis returns an error for cursor params — non-trivial with score-only sorted sets).get_signatures_for_address_impl.rs: address validation, limit clamped to[1, 1000], typedRpcSignaturesForAddressConfig.KNOWN_RPC_METHODSandACCOUNT_GATED_METHODS(JWT required, Phase 2 ownership check for user-role callers).Tests
core/src/accounts/traits.rscore/src/rpc/mod.rsgateway/src/auth.rs+gateway/tests/auth_integration.rsNotes
mint.rsidempotency check now actually works (previously hit-32601 method not found).Coverage Report