get-external-tx-optimisations #255
Conversation
|
🤖 Claude Code Review Status: Complete Critical Issue Found: This PR introduces a breaking bug by removing the automatic field dependency logic from See inline comments for details: Recommendation: Restore the removed Positive aspects:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
stores/utxo/aerospike/get.go
Outdated
| func parseInputReferencesOnly(reader io.Reader) ([]*bt.Input, error) { | ||
|
|
||
| // Parse version (4 bytes) | ||
| var version uint32 |
There was a problem hiding this comment.
Fixed - The code now correctly uses io.CopyN(io.Discard, reader, 4) to skip the version bytes without assigning to an unused variable.
There was a problem hiding this comment.
✅ Confirmed fixed in latest commit. The code now correctly uses io.CopyN(io.Discard, reader, 4) at line 1455.
…o stu/get-external-tx-optimisations
|
…d Extended needed if TxInpoints is specified
| } | ||
| } | ||
|
|
||
| if slices.Contains(newBins, fields.Tx) { |
There was a problem hiding this comment.
Critical Bug: The removed logic here (lines 456-459 in previous version) automatically added fields.Inputs and fields.External when TxInpoints was requested. Without this, callers using utxo.MetaFields will fail on external transactions because bins will not contain the required data. Fix: Restore the deleted TxInpoints check or remove TxInpoints from utxo.MetaFields.
| // Default fields - optimized for common use cases without scripts | ||
| // Services that need full transaction (persister, API) explicitly request fields.Tx | ||
| // TxInpoints requires Inputs and External fields to be fetched | ||
| bins := []fields.FieldName{fields.Fee, fields.SizeInBytes, fields.TxInpoints, fields.Inputs, fields.External, fields.BlockIDs, fields.IsCoinbase} |
There was a problem hiding this comment.
Inconsistency: This comment states "TxInpoints requires Inputs and External fields to be fetched" but the default bins list below manually includes these fields. The removed addAbstractedBins logic was supposed to add these automatically. Now callers who explicitly pass item.Fields (line 534-535) will bypass this default and miss the required dependencies.
1 participant
No description provided.