Port Syscoin support to latest upstream Blockbook#23
Conversation
Reapply Syscoin SPT parsing, indexing, API, explorer, packaging, and test fixtures on top of current upstream Blockbook so the fork can review Syscoin-specific changes separately from the main replacement. 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: 601e699686
ℹ️ 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 sender, ok := chain.(bchain.SendRawTransactionOpts); ok && (p.MaxFeeRate != nil || p.MaxBurnAmount != nil) { | ||
| return sender.SendRawTransactionWithOpts(p) | ||
| } | ||
| return hex.String(), nil | ||
| return chain.SendRawTransaction(p.Hex, p.DisableAlternativeRPC) |
There was a problem hiding this comment.
When Blockbook is created via coins.NewBlockChain, s.chain is a *blockChainWithMetrics. That wrapper only exposes SendRawTransaction, not SendRawTransactionWithOpts, so this assertion is false in production; requests with maxfeerate/maxburnamount fall through to SendRawTransaction and SyscoinRPC replaces the caller-supplied values with its defaults. Users therefore cannot control the burn allowance through the new REST, explorer, or WebSocket parameters unless the wrapper forwards the opt-in method or this helper unwraps it.
Useful? React with 👍 / 👎.
| assetMempool, ok := w.mempool.(interface { | ||
| GetTxAssets(uint64) bchain.MempoolTxidEntries | ||
| }) | ||
| if !ok { | ||
| return txids, nil |
There was a problem hiding this comment.
For /api/v2/asset requests with unconfirmed SPT transactions, w.mempool is the metrics wrapper returned by coins.NewBlockChain, and neither that wrapper nor the base mempool implements GetTxAssets; this assertion therefore fails and returns an empty list. As a result, unconfirmedTxs, unconfirmedBalance, and mempool transaction rows are always missing from asset responses/pages even when the mempool contains transfers for the asset. Please implement and forward an asset mempool index, or derive these entries from the existing mempool data.
Useful? React with 👍 / 👎.
Keep Syscoin asset mempool lookups indexed through BaseMempool and forward both asset lookup and optional sendrawtransaction parameters through upstream metrics wrappers. 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: eb9cb7b949
ℹ️ 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".
| } | ||
| filterLower := strings.ToLower(strings.ReplaceAll(filter, "0x", "")) | ||
| assetDetails := make([]*AssetsSpecific, 0) | ||
| for guid, assetCached := range *w.db.GetAssetCache() { |
There was a problem hiding this comment.
When /api/v2/assets/<filter> returns more matches than one page, FindAssets pages the slice built here from a Go map returned by GetAssetCache. Map iteration order is randomized, so the same search can reshuffle between requests and make paged clients see duplicate assets or skip assets entirely. Sort the collected results by a stable key such as GUID or symbol before calling computePaging.
Useful? React with 👍 / 👎.
| if !b.ParseBlocks { | ||
| return b.GetBlockFull(hash) | ||
| } | ||
| return b.GetBlockWithoutHeader(hash, height) |
There was a problem hiding this comment.
When callers supply a block hash with height == 0 (the standard by-hash GetBlock path, e.g. Chain.GetBlock(hash, 0)), this always calls GetBlockWithoutHeader(hash, 0), which stamps the returned block with height 0 and omits the header metadata that BitcoinRPC fetches in this case. Syscoin block lookups by hash therefore return incorrect height/prev/next data; keep the GetBlockWithoutHeader optimization only when the height is known and fetch the header otherwise.
Useful? React with 👍 / 👎.
Sort Syscoin asset search results before paging and preserve upstream header lookup behavior when blocks are requested by hash without a known height. Co-authored-by: Cursor <cursoragent@cursor.com>
Add the Syscoin 5.1 release checksum signature metadata to the testnet backend config so the upstream artifact lint accepts both default and arm64 downloads. Co-authored-by: Cursor <cursoragent@cursor.com>
Remove the Syscoin GetBlock override now that the embedded BitcoinRPC already uses the Syscoin parser and upstream header lookup behavior. 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: 8388f45aa7
ℹ️ 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".
| vin.AddrDesc = addrDesc | ||
| vin.ValueSat = *value | ||
| return &addrIndex{string(addrDesc), ^int32(vin.Vout)} | ||
| return &addrIndex{addrDesc: string(addrDesc), n: ^int32(vin.Vout), AssetInfo: assetInfo} |
There was a problem hiding this comment.
When a mempool transaction spends a confirmed Syscoin SPT UTXO, AddrDescForOutpoint resolves the prevout before the code fetches the parent transaction/cache, so assetInfo remains nil here. That means vin.AssetInfo and the mempool addrIndex do not carry the spent asset, and address/asset mempool views fail to subtract outgoing confirmed-token spends until the transaction confirms. Please carry the prevout AssetInfo on this fast path or fall through to the stored tx-address data for Syscoin assets.
Useful? React with 👍 / 👎.
| totalResults := int(dbAsset.Transactions) | ||
| if filter.FromHeight != 0 || filter.ToHeight != 0 { | ||
| totalResults = -1 |
There was a problem hiding this comment.
When an asset request uses assetMask (for example ?assetMask=token-transfers), getAssetTxids filters the returned rows by mask, but totalResults still uses the unfiltered dbAsset.Transactions count unless a height range is present. The pagination code then advertises total pages based on all asset transactions, so filtered asset histories can show extra/empty pages. Mark the total as unknown or compute the filtered count whenever filter.AssetsMask != bchain.AllMask.
Useful? React with 👍 / 👎.
Carry indexed SPT asset metadata through mempool input lookups and mark asset-mask filtered histories as unknown-sized for paging. 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: 839a438409
ℹ️ 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".
| rawMarshal, err := json.Marshal(&res.Result) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| decodedRawString := string(rawMarshal) |
There was a problem hiding this comment.
When syscoingetspvproof returns a JSON string proof, res.Result already contains the JSON bytes; marshalling that json.RawMessage and then placing it in the API's string result field adds another encoding layer, so clients receive escaped quotes (for example "proof") instead of the proof itself. Object/array results would likewise be returned as escaped JSON text rather than the original payload, which breaks bridge consumers expecting the RPC proof shape.
Useful? React with 👍 / 👎.
| if filterParam != "" { | ||
| data.PageParams = template.URL("&assetMask=" + filterParam) | ||
| data.Asset.Filter = filterParam |
There was a problem hiding this comment.
For /asset/{guid}?assetMask=token-transfers, getAddressQueryParams correctly parses the asset mask into filter, but filterParam comes from the unrelated filter query key. This branch is therefore skipped for the asset page's own dropdown, so pagination links drop assetMask and the selected filter is not reflected; clicking a later page silently resets the history back to all asset transactions.
Useful? React with 👍 / 👎.
| if mask == bchain.AllMask { | ||
| return true, nil | ||
| } | ||
| return uint32(filter.AssetsMask)&uint32(mask) == uint32(mask), nil |
There was a problem hiding this comment.
This new asset-mask predicate filters the returned address txids, but GetAddress still treats totalResults as the unfiltered ba.Txs whenever there is no height/vout filter. On Syscoin address API/explorer requests such as ?assetMask=token-only, pagination can be computed from all address transactions instead of the filtered subset, producing inflated totalPages and empty pages; mark AssetsMask != AllMask as an unknown/filtered total or count the filtered results.
Useful? React with 👍 / 👎.
| symbol = fmt.Sprintf("ERC20-%d", assetId) | ||
| } | ||
| metadata = "ERC20 Token" | ||
| precision = 8 |
There was a problem hiding this comment.
When the vault registry reports an ERC20 asset with a precision other than 8, this overwrite discards the on-chain registry.Precision value that was just read and stores/display amounts with 8 decimals. That makes balances, supplies, and explorer formatting wrong for bridged ERC20 assets whose Syscoin precision differs from 8; keep the registry precision for ERC20 and only force zero for NFT-style assets.
Useful? React with 👍 / 👎.
Return SPV proof payloads without double encoding, preserve asset masks in explorer paging, treat asset-mask address histories as filtered, and keep registry precision for ERC20 assets. Co-authored-by: Cursor <cursoragent@cursor.com>
Carry syscoingetspvproof results as json.RawMessage so object proofs are returned without string escaping. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
Restore 8-decimal display precision for ERC20-backed Syscoin assets because SPT amounts are indexed as Syscoin CAmount values. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 638ac05255
ℹ️ 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".
| dBAsset, err := d.GetAsset(assetInfo.AssetGuid, assets) | ||
| if err != nil { | ||
| glog.Warningf("ConnectAllocationOutput using fallback asset %d: %v", assetInfo.AssetGuid, err) | ||
| dBAsset = fallbackSyscoinAsset(assetInfo.AssetGuid) |
There was a problem hiding this comment.
When NEVM metadata lookup fails transiently while indexing the first transaction for an asset, this fallback object is later put into the per-block assets map and persisted by storeAssets; subsequent GetAsset calls hit the cache/DB fallback and never retry FetchNEVMAssetDetails. That makes a short NEVM outage permanently expose numeric placeholder symbols/precision for the asset until the DB/cache is manually repaired.
Useful? React with 👍 / 👎.
| vin.AddrDesc = addrDesc | ||
| vin.ValueSat = *value | ||
| return &addrIndex{string(addrDesc), ^int32(vin.Vout)} | ||
| return &addrIndex{addrDesc: string(addrDesc), n: ^int32(vin.Vout), AssetInfo: assetInfo} |
There was a problem hiding this comment.
When a mempool Syscoin transaction spends an existing SPT UTXO, the resolved assetInfo is only copied into the private addrIndex; the MempoolVin itself remains without AssetInfo, and websocket OnNewTx converts that same MempoolTx via GetTransactionFromMempoolTx. As a result new-transaction websocket payloads for asset spends omit vin.assetInfo even though confirmed/full transaction responses include it, leaving subscribers unable to see the input-side SPT amount.
Useful? React with 👍 / 👎.
Retry NEVM metadata when cached asset placeholders are encountered and include SPT input metadata in mempool websocket transaction conversion. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review, note spt are 8 decimals for erc20 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2b23fb895
ℹ️ 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".
| BlockHeader: bchain.BlockHeader{ | ||
| Size: len(b), | ||
| Time: h.Timestamp.Unix(), | ||
| }, |
There was a problem hiding this comment.
When Syscoin sync uses parsed raw blocks, BitcoinRPC.GetBlockWithoutHeader only fills Hash and Height after ParseBlock, and db/sync.go only detects forks when block.Prev is non-empty. This parser reads the header into h but drops h.PrevBlock, so normal height-based Syscoin sync will skip reorg/fork detection and may continue indexing on the wrong parent; set Prev: h.PrevBlock.String() as the other raw/auxpow block parsers do.
Useful? React with 👍 / 👎.
| if page == 0 { | ||
| if option == AccountDetailsTxidHistory { | ||
| txids = append(txids, tx.Txid) | ||
| } else if option >= AccountDetailsTxHistoryLight { | ||
| txs = append(txs, tx) | ||
| } |
There was a problem hiding this comment.
For page 1 of an active asset, every mempool transaction is appended here before confirmed history, but the confirmed query below still adds a full page and there is no final truncation like GetAddress performs. With M pending asset transactions and pageSize=N, /api/v2/asset/... can return M+N txs/txids and ignore the caller's page size, making hot assets produce unexpectedly large responses; trim after prepending mempool entries or reduce the confirmed slice accordingly.
Useful? React with 👍 / 👎.
Preserve previous block hashes for Syscoin raw block parsing and keep asset history responses bounded when mempool rows are prepended. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review, note spt for erc20 always 8 decimals |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f9f77b622
ℹ️ 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".
| default: | ||
| symbol = fmt.Sprintf("UNKNOWN-%d", assetId) | ||
| metadata = "Unknown Asset Type" |
There was a problem hiding this comment.
When an asset GUID is not already indexed, GetAsset falls back to FetchNEVMAssetDetails; for an unregistered assetRegistry key the vault call returns the zero-valued struct (AssetType == 0), but this default branch still constructs and caches an UNKNOWN-* asset. That makes /api/v2/asset/<unused-guid> succeed with fake SPT metadata instead of returning "Asset not found", and the bogus row can later appear in asset search from the cache. Return an error for zero/unsupported asset types instead of fabricating metadata.
Useful? React with 👍 / 👎.
| <h3 class="col-sm-6 col-lg-3 m-0 align-self-center">Transactions</h3> | ||
| <div class="col-sm-6 col-lg-3 my-2 my-lg-0 align-self-center"> | ||
| <select class="w-100" onchange="self.location='?filter='+options[selectedIndex].value"> | ||
| {{if $addr.TokensAsset}}<select class="w-100" onchange="self.location='?assetMask='+options[selectedIndex].value+'&filter='+options[selectedIndex].value">{{else}}<select class="w-100" onchange="self.location='?filter='+options[selectedIndex].value">{{end}} |
There was a problem hiding this comment.
When an address has SPT assets, this selector navigates with both assetMask and filter, but explorerAddress only copies filter into PageParams for Next/Previous links. For the new Syscoin options such as token-only or non-tokens, filter is not parsed as an asset mask, so page 2+ drops the assetMask and shows unfiltered transactions while the option still appears selected. Include assetMask in the pagination params for these options.
Useful? React with 👍 / 👎.
Avoid caching fabricated NEVM asset metadata for unsupported registry entries and keep Syscoin assetMask filters on address pagination links. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review reminder spt erc20s are 8 decimal |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d43219ae8d
ℹ️ 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".
| valueStr := "" | ||
| if assetInfo.ValueSat != nil { | ||
| if dbAsset, err := w.db.GetAsset(assetInfo.AssetGuid, nil); err == nil { | ||
| valueStr = value.DecimalString(int(dbAsset.AssetObj.Precision)) + " " + string(dbAsset.AssetObj.Symbol) |
There was a problem hiding this comment.
When asset metadata is available, this makes assetInfo.valueStr include a symbol suffix such as 1.00000000 SYSX, while the fallback path returns only the numeric decimal string and the new Syscoin API notes describe valueStr as a decimal amount. Clients that parse valueStr as an amount will fail for the normal indexed-asset case; keep the symbol in its own field/display layer.
Useful? React with 👍 / 👎.
| if tx.Confirmations != 0 { | ||
| continue | ||
| } | ||
| uBalSat.Add(&uBalSat, tx.assetOutValue(asset)) |
There was a problem hiding this comment.
For /asset responses that include mempool transactions, this adds only the output-side asset amount. A pending SPT transfer therefore reports a positive unconfirmedBalance even though total asset supply is unchanged, and a pending burn reports zero instead of a negative delta; use the vin assetInfo amounts as well, like the address mempool accounting does.
Useful? React with 👍 / 👎.
Keep asset valueStr numeric-only and compute unconfirmed asset balances from both mempool outputs and inputs. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep assetInfo valueStr numeric while exposing asset symbols separately for transaction detail display. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review, current head keeps assetInfo.valueStr numeric and exposes assetInfo.symbol separately for display |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c3f6a0887
ℹ️ 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".
| }, nil | ||
| } | ||
|
|
||
| ctx := context.Background() |
There was a problem hiding this comment.
When indexing or serving an asset whose metadata is missing from RocksDB, this path calls the NEVM RPC with an unbounded background context (and the helper calls for symbols/token IDs do the same). If the local sysgeth HTTP endpoint accepts the request but stalls, the primary call can block the block-connect or API goroutine indefinitely and never fall back to the backup client; derive a context with the configured RPC timeout for these enrichment calls.
Useful? React with 👍 / 👎.
| } | ||
| ro := gorocksdb.NewDefaultReadOptions() | ||
| ro.SetFillCache(false) | ||
| it := d.db.NewIteratorCF(d.ro, d.cfh[cfAssets]) |
There was a problem hiding this comment.
On the first Syscoin asset search, SetupAssetCache scans the entire cfAssets column family, but the iterator still uses d.ro instead of the read options configured with SetFillCache(false) just above. On instances with many assets this warms/evicts the normal RocksDB block cache during a full metadata scan, and the allocated options are also never destroyed; pass ro to the iterator and destroy it after the scan.
Useful? React with 👍 / 👎.
Apply configured RPC timeouts to NEVM metadata calls and use non-caching RocksDB read options for full asset cache scans. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review, latest head bounds NEVM metadata RPC calls and uses uncached read options for asset cache scans |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6fde0f701c
ℹ️ 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 p, ok := c.b.(interface { | ||
| FetchNEVMAssetDetails(uint64) (*bchain.Asset, error) | ||
| }); ok { | ||
| defer func(s time.Time) { c.observeRPCLatency("FetchNEVMAssetDetails", s, nil) }(time.Now()) |
There was a problem hiding this comment.
When a Syscoin asset metadata lookup times out or the NEVM RPC returns an error, this wrapper still calls observeRPCLatency with a nil error, so the RPC metrics report failed FetchNEVMAssetDetails calls as successes. That makes the new bounded NEVM fallback path much harder to alert on or diagnose when the API/indexer is returning fallback metadata or Asset not found due to RPC failures.
Useful? React with 👍 / 👎.
Pass FetchNEVMAssetDetails errors through the metrics wrapper so failed metadata RPC calls are observable. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review, latest head records FetchNEVMAssetDetails errors in RPC metrics |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74f359f5b7
ℹ️ 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".
| "backend_rpc": 18035, | ||
| "backend_message_queue": 48335, | ||
| "blockbook_internal": 19135, | ||
| "blockbook_public": 19035 |
There was a problem hiding this comment.
When syscoin_testnet is deployed on the same host as the existing bgold_testnet, these ports collide: configs/coins/bgold_testnet.json already uses backend RPC 18035, ZMQ 48335, and Blockbook ports 19035/19135 (with internal/public swapped). The generated services will try to bind the same RPC/ZMQ/HTTP ports, so one of the testnet instances cannot start; please choose an unused port block for the new coin.
Useful? React with 👍 / 👎.
Move Syscoin testnet to an unused port block so it no longer collides with bgold testnet services. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review, latest head assigns Syscoin testnet to unused 67 port block |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e85c778bc
ℹ️ 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".
| "--http.addr", | ||
| "127.0.0.1", | ||
| "--http.port", | ||
| "8545", |
There was a problem hiding this comment.
When Syscoin is deployed on the same host as Tron, this managed sysgeth --http.port collides with the existing Tron backend port: configs/coins/tron.json and configs/coins/tron_testnet_nile.json both bind their backend RPC/httpFullNodePort to 8545. Because Blockbook also dials Syscoin metadata via web3_rpc_url on this same port, one of the services will fail to bind or Blockbook may talk to the wrong local backend; assign Syscoin's NEVM HTTP endpoint to an unused Syscoin-specific port and keep web3_rpc_url in sync.
Useful? React with 👍 / 👎.
| varBuf := make([]byte, 4) | ||
| l := p.BaseParser.PackVaruint(uint(asset.Transactions), varBuf) |
There was a problem hiding this comment.
If a long-lived/high-traffic asset's Transactions count ever needs five VLQ bytes, this four-byte scratch buffer is too small for PackVaruint(uint(asset.Transactions), varBuf) and will panic while storing asset metadata, stopping sync for that block. Use the VLQ max-size buffer here, as the surrounding asset/key serializers do, so the full uint32 counter can be encoded safely.
Useful? React with 👍 / 👎.
Move Syscoin's managed NEVM HTTP endpoint off Tron's shared port and size the asset transaction-count VLQ buffer for the full encoded range. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review, latest head moves Syscoin NEVM HTTP off Tron 8545 and fixes asset transaction-count VLQ buffer |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eeb32c976a
ℹ️ 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".
| symbol = fmt.Sprintf("ERC20-%d", assetId) | ||
| } | ||
| metadata = "ERC20 Token" | ||
| precision = 8 |
There was a problem hiding this comment.
For bridged ERC20 assets whose assetRegistry(...).precision is not 8, this overwrites the precision already read from NEVM with a hard-coded value. The API/explorer later uses AssetObj.Precision to format balances and token rows, so those assets will show amounts with the wrong decimal scale even though the registry supplied the correct precision.
Useful? React with 👍 / 👎.
| Symbol string `json:"symbol" ts_doc:"Asset symbol."` | ||
| TotalSupply *Amount `json:"totalSupply" ts_doc:"Current total supply in base units."` | ||
| Decimals int `json:"precision" ts_doc:"Number of asset decimal places."` | ||
| Txs int `json:"Txs" ts_doc:"Number of indexed transactions touching this asset."` |
There was a problem hiding this comment.
The /api/v2/assets/... response serializes the compact asset transaction count as "Txs", while the detailed asset response and the rest of the Blockbook API expose transaction counts as "txs". Clients consuming asset-search results will miss the count if they read the documented/lowercase field, so this should use json:"txs" for consistency.
Useful? React with 👍 / 👎.
Expose lowercase txs on asset-search rows while preserving the legacy Txs field for existing Syscoin clients, and document the intentional 8-decimal UTXO asset formatting. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review, latest head adds lowercase txs while preserving legacy Txs and documents intentional 8-decimal Syscoin UTXO asset formatting |
Keep asset-search transaction count consistent with the rest of the API by exposing only the lowercase txs field. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review, latest head uses lowercase txs only for asset-search rows after verifying Pali's active SPT sync paths do not consume legacy Txs |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d2eb26d17
ℹ️ 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 err != nil { | ||
| return nil, 0, err | ||
| } | ||
| p.LoadAssets(tx) |
There was a problem hiding this comment.
When a transaction has one of the Syscoin asset versions but LoadAssets cannot decode its allocation OP_RETURN, this call drops the error and still returns the transaction. That lets block or mempool sync continue with no AssetInfo on the outputs, so SPT balances and asset history are silently indexed as if the asset movement did not happen; please return the error from the parsing paths instead of ignoring it.
Useful? React with 👍 / 👎.
Return SPT allocation decode errors from Syscoin transaction and block parsing so malformed asset transactions cannot be indexed without AssetInfo. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review, latest head propagates Syscoin SPT allocation parse errors from raw tx, JSON tx, unpacked tx, and block parsing |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9bc784919c
ℹ️ 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".
| } else if op == txscript.OP_PUSHDATA1 { | ||
| data = script[3:] | ||
| } else if op == txscript.OP_PUSHDATA2 { | ||
| data = script[4:] |
There was a problem hiding this comment.
For an asset-version transaction with a malformed OP_RETURN such as 6a4c or 6a4d00, these slices go past the end of the script and panic before LoadAssets can return an error. Since raw tx, JSON tx, unpacked tx, and block parsing all route through this helper, one malformed SPT payload can crash the parsing path instead of being reported as a parse failure.
Useful? React with 👍 / 👎.
| for _, v := range allocation.AssetObj.VoutAssets { | ||
| for _, voutAsset := range v.Values { | ||
| // store in vout | ||
| tx.Vout[voutAsset.N].AssetInfo = &bchain.AssetInfo{AssetGuid: v.AssetGuid, ValueSat: big.NewInt(voutAsset.ValueSat)} |
There was a problem hiding this comment.
When the decoded SPT allocation contains a voutAsset.N that is not present in tx.Vout, this direct index panics instead of returning an allocation parse error. The allocation payload is transaction data, so malformed raw/JSON/unpacked/block inputs can still bring down the parser even though the surrounding code now propagates LoadAssets errors.
Useful? React with 👍 / 👎.
Validate SPT OP_RETURN pushdata lengths and allocation output indexes so malformed asset transactions return parser errors instead of panicking. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review, latest head validates SPT OP_RETURN pushdata lengths and allocation vout indexes so malformed asset payloads return errors instead of panicking |
|
Codex Review: Didn't find any major issues. Keep it up! 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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f060b11554
ℹ️ 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".
| return &bchain.Asset{ | ||
| Transactions: transactions, | ||
| AssetObj: syscoinwire.AssetType{ | ||
| Contract: make([]byte, 20), |
There was a problem hiding this comment.
For the built-in SYSX asset (assetGuid 123456), this 20-byte zero Contract is later treated as a real NEVM contract because GetAsset/FindAssetsFromFilter convert any non-empty contract byte slice with BytesToAddress. As a result the SYSX asset API/search/explorer pages expose and link to 0x0000000000000000000000000000000000000000, even though the NEVM fetch special case returns an empty contract for the same native asset. Leave this empty/nil so SYSX does not advertise a bogus contract.
Useful? React with 👍 / 👎.
Keep the built-in SYSX asset contract empty so native SYSX does not expose or link to the zero NEVM address while preserving canonical metadata. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review, latest head leaves built-in SYSX contract empty while preserving canonical SYSX metadata and transaction count |
|
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
Test plan
git diff --check && git diff --cached --checkgo test ./bchain ./bchain/coins/btc ./bchain/coins/sys/api/v2/getspvproof/{txid}returns a non-empty proof for a mainnet SYSX transaction.Made with Cursor