-
Notifications
You must be signed in to change notification settings - Fork 8
feat: Introduce bloom filter sync worker, optimize Bitcoin RPC prevout resolution, and persist block hashes for reorg detection. #43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import ( | |
| "context" | ||
| "encoding/json" | ||
| "fmt" | ||
| "sync" | ||
| "time" | ||
|
|
||
| "github.com/fystack/multichain-indexer/internal/rpc" | ||
|
|
@@ -176,30 +177,113 @@ func (c *BitcoinClient) GetTransactionWithPrevouts(ctx context.Context, txid str | |
| return nil, err | ||
| } | ||
|
|
||
| // If prevout data already present, return as-is | ||
| if len(tx.Vin) > 0 && tx.Vin[0].PrevOut != nil { | ||
| return tx, nil | ||
| if err := c.ResolvePrevouts(ctx, []*Transaction{tx}, 4); err != nil { | ||
| return nil, err | ||
| } | ||
| return tx, nil | ||
| } | ||
|
|
||
| // Resolve prevout for each input (skip coinbase) | ||
| for i := range tx.Vin { | ||
| if tx.Vin[i].TxID == "" { | ||
| continue // Coinbase input | ||
| } | ||
| // ResolvePrevouts resolves prevout data for all inputs across multiple transactions | ||
| // using parallel fetching with deduplication. This eliminates the N+1 problem where | ||
| // each input would otherwise require a separate RPC call. | ||
| func (c *BitcoinClient) ResolvePrevouts(ctx context.Context, txs []*Transaction, concurrency int) error { | ||
| if concurrency <= 0 { | ||
| concurrency = 8 | ||
| } | ||
|
|
||
| prevTx, err := c.GetRawTransaction(ctx, tx.Vin[i].TxID, true) | ||
| if err != nil { | ||
| // Log but don't fail - some prevouts may be unavailable | ||
| // Collect all unique prevout txids we need to fetch | ||
| type prevoutRef struct { | ||
| txid string | ||
| vout uint32 | ||
| } | ||
| needed := make(map[string]struct{}) | ||
| for _, tx := range txs { | ||
| if tx.IsCoinbase() { | ||
| continue | ||
| } | ||
| // Skip if prevouts are already resolved | ||
| if len(tx.Vin) > 0 && tx.Vin[0].PrevOut != nil { | ||
| continue | ||
| } | ||
| for _, vin := range tx.Vin { | ||
| if vin.TxID != "" { | ||
| needed[vin.TxID] = struct{}{} | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if len(needed) == 0 { | ||
| return nil | ||
| } | ||
|
|
||
| // Fetch all needed transactions in parallel with bounded concurrency | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we only need to monitor tx where "to" addresses are monitored, why ResolvePrevouts needs fetches the entire previous transaction?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bitcoin UTXO model - inputs only have (txid, vout) reference, no address or amount. We need to fetch the full prev tx to get the from-address and calculate fees. Unfortunately getrawtransaction is the only RPC available for this. I think it should better track full UTXO record for future scaling.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agreed with you on this point, and i have a pending PR for indexing UTXO: #42 FYI, This indexer only track tx where "to" is on monitored list, but for the UTXO, is it good if we tracking both direction? Because every tx can have multiple direction of UTXO? I'd appriciate if you can take a look in this and give me some opinion, |
||
| var mu sync.Mutex | ||
| prevoutCache := make(map[string]*Transaction, len(needed)) | ||
|
|
||
| txids := make([]string, 0, len(needed)) | ||
| for txid := range needed { | ||
| txids = append(txids, txid) | ||
| } | ||
|
|
||
| jobs := make(chan string, concurrency*2) | ||
| var wg sync.WaitGroup | ||
|
|
||
| for i := 0; i < concurrency; i++ { | ||
| wg.Add(1) | ||
| go func() { | ||
| defer wg.Done() | ||
| for txid := range jobs { | ||
| prevTx, err := c.GetRawTransaction(ctx, txid, true) | ||
| if err != nil { | ||
| continue // Skip unavailable prevouts | ||
Azzurriii marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| mu.Lock() | ||
| prevoutCache[txid] = prevTx | ||
| mu.Unlock() | ||
| } | ||
| }() | ||
| } | ||
|
|
||
| go func() { | ||
| defer close(jobs) | ||
| for _, txid := range txids { | ||
| select { | ||
| case <-ctx.Done(): | ||
| return | ||
| case jobs <- txid: | ||
| } | ||
| } | ||
| }() | ||
|
|
||
| voutIdx := tx.Vin[i].Vout | ||
| if int(voutIdx) < len(prevTx.Vout) { | ||
| tx.Vin[i].PrevOut = &prevTx.Vout[voutIdx] | ||
| wg.Wait() | ||
| if ctx.Err() != nil { | ||
| return ctx.Err() | ||
| } | ||
|
|
||
| // Assign resolved prevouts back to inputs | ||
| for _, tx := range txs { | ||
| if tx.IsCoinbase() { | ||
| continue | ||
| } | ||
| if len(tx.Vin) > 0 && tx.Vin[0].PrevOut != nil { | ||
| continue | ||
| } | ||
| for i := range tx.Vin { | ||
| if tx.Vin[i].TxID == "" { | ||
| continue | ||
| } | ||
| prevTx, ok := prevoutCache[tx.Vin[i].TxID] | ||
| if !ok { | ||
| continue | ||
| } | ||
| voutIdx := tx.Vin[i].Vout | ||
| if int(voutIdx) < len(prevTx.Vout) { | ||
| tx.Vin[i].PrevOut = &prevTx.Vout[voutIdx] | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return tx, nil | ||
| return nil | ||
| } | ||
|
|
||
| // GetMempoolEntry returns mempool entry for a specific transaction | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be configurable, or define a const instead of using a magic number
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll extract this to a const DefaultPrevoutConcurrency = 8 and also fix the hardcoded 8 in GetMempoolTransactions. The value itself comes from the caller, convertBlockWithPrevoutResolution already passes config.Throttle.Concurrency, so the default here is only a safety fallback.