feat(syscoin): add compact tx summary history#27
Conversation
Expose a Syscoin-specific account history mode for wallet transaction lists so clients can avoid storing full vin/vout payloads while still showing account-relative amounts and SPT deltas. Co-authored-by: Cursor <cursoragent@cursor.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56006936f9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Details string `json:"details,omitempty" ts_type:"'basic' | 'tokens' | 'tokenBalances' | 'txids' | 'txslight' | 'txs'" ts_doc:"Level of detail to retrieve about the account."` | ||
| Descriptor string `json:"descriptor" ts_doc:"Address or XPUB descriptor to query."` | ||
| // SYSCOIN: txsummary is a compact account-context history mode for wallet lists. | ||
| Details string `json:"details,omitempty" ts_type:"'basic' | 'tokens' | 'tokenBalances' | 'txids' | 'txsummary' | 'txslight' | 'txs'" ts_doc:"Level of detail to retrieve about the account."` |
There was a problem hiding this comment.
This advertises details: "txsummary" for the getAccountInfo websocket request, but getAccountInfo in server/websocket.go has no matching case and falls through to AccountDetailsBasic. A WS client following this generated type/doc will get only basic account data instead of the compact transaction summaries, so the new history mode is unavailable on the surface where it is documented.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2bae226: websocket getAccountInfo now maps details=txsummary to AccountDetailsTxHistorySummary.
| case "txsummary": | ||
| accountDetails = api.AccountDetailsTxHistorySummary |
There was a problem hiding this comment.
This shared query parser is also used by apiAsset, so /api/v2/asset/...?...details=txsummary now passes AccountDetailsTxHistorySummary into GetAsset. There, unconfirmed asset txs are counted for every option >= AccountDetailsTxidHistory, but mempoolTxs is populated only for options >= AccountDetailsTxHistoryLight; because summary sits between them, any asset with mempool transactions can slice an empty mempoolTxs slice and panic. Either reject this detail level for asset routes or handle it like the tx history modes there.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2bae226: apiAsset now rejects details=txsummary before calling GetAsset, since txsummary is account-context only.
Wire txsummary through websocket account info requests and reject it on asset history endpoints where account-context summaries do not apply. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2bae2269a7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // only ChainBitcoinType supports TxHistoryLight | ||
| if option == AccountDetailsTxHistoryLight && w.chainType == bchain.ChainBitcoinType { | ||
| // SYSCOIN: summary history shares the indexed Bitcoin-type light path. | ||
| if option >= AccountDetailsTxHistorySummary && option <= AccountDetailsTxHistoryLight && w.chainType == bchain.ChainBitcoinType { |
There was a problem hiding this comment.
When details=txsummary is used on confirmed Bitcoin/Syscoin history, this branch now reuses the light txFromTxAddress path, but that converter only fills Addresses and never copies AddrDesc into each Vin/Vout. summarizeTxForAccount matches exclusively on vin.AddrDesc/vout.AddrDesc, so confirmed summaries from this path report zero account input/output, no asset transfers, and default to received; mempool summaries still work because they come from getTransaction. Please either populate descriptors in the light conversion or summarize against the indexed TxAddresses descriptors.
Useful? React with 👍 / 👎.
Keep indexed input/output descriptors when building light transaction rows so compact account summaries can compute confirmed account-relative values. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f63cc7c563
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case "txsummary": | ||
| accountDetails = api.AccountDetailsTxHistorySummary |
There was a problem hiding this comment.
Because this generic parser accepts details=txsummary for every coin, Ethereum-like address endpoints can now reach summarizeTxForAccount; for a sender, Ethereum Vin entries have the address descriptor but no ValueSat, so the summary reports zero in/out and direction: "received" instead of the sent value. The WebSocket mapping has the same exposure, so this mode should be rejected outside the Syscoin/UTXO path or implemented for EVM transactions before advertising it.
Useful? React with 👍 / 👎.
| transfers = append(transfers, TokenTransfer{ | ||
| AssetGuid: guid, | ||
| Symbol: summary.symbol, | ||
| Decimals: summary.decimals, | ||
| Value: (*Amount)(value), |
There was a problem hiding this comment.
These summary transfers are returned both as accountAssetTransfers and as TokenTransfers, but the literal leaves the non-omitempty type, standard, and contract fields at empty strings. Existing Syscoin transfer rows from getSyscoinAssetTransfers identify SPT transfers with SPT and the asset GUID as the contract; without the same fields here, clients consuming the normal token-transfer shape cannot reliably classify or key summary SPT movements.
Useful? React with 👍 / 👎.
Reject txsummary on non-UTXO account endpoints and populate SPT identity fields in compact account transfer summaries. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05085d7a5c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Details string `json:"details,omitempty" ts_type:"'basic' | 'tokens' | 'tokenBalances' | 'txids' | 'txslight' | 'txs'" ts_doc:"Level of detail to retrieve about the account."` | ||
| Descriptor string `json:"descriptor" ts_doc:"Address or XPUB descriptor to query."` | ||
| // SYSCOIN: txsummary is a compact account-context history mode for wallet lists. | ||
| Details string `json:"details,omitempty" ts_type:"'basic' | 'tokens' | 'tokenBalances' | 'txids' | 'txsummary' | 'txslight' | 'txs'" ts_doc:"Level of detail to retrieve about the account."` |
There was a problem hiding this comment.
This exposes details='txsummary' in the WebSocket request type, but the checked-in API schemas were not updated: blockbook-api.ts still types details as only 'basic' | 'tokens' | 'tokenBalances' | 'txids' | 'txslight' | 'txs', and openapi.yaml has the same enum without txsummary; the new summary Tx fields are also absent there. Clients generated from those canonical files cannot discover or type-check this new mode even though the server accepts it.
Useful? React with 👍 / 👎.
Expose compact Syscoin transaction summary fields and the txsummary detail option in checked-in API schema artifacts. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce952d2777
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // SYSCOIN | ||
| func (s *PublicServer) validateTxSummarySupported(r *http.Request) error { | ||
| if r.URL.Query().Get("details") == "txsummary" && !s.supportsAccountTxSummary() { |
There was a problem hiding this comment.
When details=txsummary is sent to /api/v1/address or /api/v1/xpub (and to unversioned /api/... on Bitcoin-type chains, where the default is v1), this helper allows the new mode because it only checks chain support. The handlers then call AddressToV1, whose TxV1 has no direction, addressValueIn/Out, or accountAssetTransfers fields and serializes empty vin/vout for compact rows, so clients receive incomplete history instead of the summary they requested. Gate txsummary on apiVersion == apiV2 or return a public error for v1.
Useful? React with 👍 / 👎.
Keep compact transaction summaries on API v2 only so v1 address conversion cannot return incomplete history rows. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9bd76b8683
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if option == AccountDetailsTxHistorySummary { | ||
| txs = append(txs, w.summarizeTxForAccount(tx, accountAddrDescs)) // SYSCOIN |
There was a problem hiding this comment.
When an address has unconfirmed transactions, page 1 prepends their summaries above and then this branch adds confirmed rows using from/to computed as if no mempool rows existed; the later page-size trim drops the extra confirmed rows, while page 2 starts at the next full confirmed page. For details=txsummary&pageSize=10 with two mempool transactions, confirmed rows 9-10 are skipped, so the confirmed window needs to be reduced/offset by the mempool count before appending summaries.
Useful? React with 👍 / 👎.
| } else if option == AccountDetailsTxHistorySummary { | ||
| txs = append(txs, w.summarizeTxForAccount(txmMap[entry.Txid], accountAddrDescs)) // SYSCOIN |
There was a problem hiding this comment.
For xpubs on the first page with unconfirmed transactions, this appends every mempool summary and the confirmed loop below still appends a full page of confirmed transactions, with no later trim in this path. As a result details=txsummary&pageSize=10 can return more than 10 rows and paging metadata that only reflects confirmed history, so clients relying on fixed page sizes get inconsistent pages when mempool entries exist.
Useful? React with 👍 / 👎.
Offset confirmed txsummary windows by first-page mempool rows so compact account history stays bounded and does not skip confirmed transactions. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a9a3c133c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| direction = "sent" | ||
| } | ||
| accountAssetTransfers := accountAssetTransfersFromSummary(assetTransfers) | ||
| return &Tx{ |
There was a problem hiding this comment.
When an account or xpub request uses details=txsummary, this constructor returns a Tx with nil Vin and Vout; those fields do not have omitempty, so the JSON contains "vin": null and "vout": null even though the public Tx schema and generated TypeScript type require arrays. Clients that iterate these fields for every transaction row can break only on the new summary mode, so initialize them to empty slices or expose a summary-specific response shape.
Useful? React with 👍 / 👎.
Initialize compact summary rows with empty vin and vout arrays so API responses continue to satisfy the public Tx schema. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 075f957df3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| AddressValueInSat: (*Amount)(&valueIn), | ||
| AddressValueOutSat: (*Amount)(&valueOut), | ||
| AccountAssetTransfers: accountAssetTransfers, | ||
| TokenType: tx.TokenType, |
There was a problem hiding this comment.
For Syscoin SPT transactions that carry a decoded OP_RETURN memo, details=txsummary now returns a compact Tx built here, but it only copies TokenType and drops tx.Memo. The regular light/full transaction paths still populate Memo, and this commit also documents Tx.memo, so wallets using the new summary history cannot display memos for exactly those SPT rows; copy Memo into the summarized transaction alongside TokenType.
Useful? React with 👍 / 👎.
Copy decoded Syscoin transaction memos into compact account summary rows so SPT history keeps memo metadata. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
details=txsummaryfor Syscoin account/xpub history responses.txids,txslight, andtxsbehavior unchanged for compatibility.Test plan
gofmt -w api/types.go api/worker.go api/xpub.go server/public.go server/ws_types.gogo test ./api ./server(blocked locally by grocksdb/RocksDB C symbol mismatch: missingrocksdb_cache_create_hyper_clockand related symbols)Made with Cursor