Ignore no op asset transfers. #1324
Conversation
Codecov Report
@@ Coverage Diff @@
## release/2.15.0 #1324 +/- ##
=================================================
Coverage ? 62.33%
=================================================
Files ? 52
Lines ? 8857
Branches ? 0
=================================================
Hits ? 5521
Misses ? 2832
Partials ? 504 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
@winder PR matches my understanding for a tactical fix. Given that a more principled change is already underway, a quick fix seems reasonable to me.
Prefer 1+ approvals from folks more familiar with go-algorand ledger to vouch that the change is sensible.
accounting/eval_preload.go
Outdated
| @@ -22,7 +22,12 @@ func addToCreatorsRequest(stxnad *transactions.SignedTxnWithAD, assetsReq map[ba | |||
| } | |||
| case protocol.AssetTransferTx: | |||
| fields := &txn.AssetTransferTxnFields | |||
| if fields.XferAsset != 0 { | |||
|
|
|||
| noOpXfer := (fields.AssetAmount == 0) && | |||
There was a problem hiding this comment.
the condition should be: not clawback (ct.AssetSender is empty), not AssetCloseTo, zero amount, sender != receiver to match apply/asset.go logic.
So I guess this needs && !fields.AssetSender.IsZero()
There was a problem hiding this comment.
Right - since we certainly grabbed the asset to check the clawback addr, even if the movement is 0 amount (I check, as I suppose we don't have to check a 0 clawback)
accounting/eval_preload.go
Outdated
| @@ -22,7 +22,12 @@ func addToCreatorsRequest(stxnad *transactions.SignedTxnWithAD, assetsReq map[ba | |||
| } | |||
| case protocol.AssetTransferTx: | |||
| fields := &txn.AssetTransferTxnFields | |||
| if fields.XferAsset != 0 { | |||
|
|
|||
| noOpXfer := (fields.AssetAmount == 0) && | |||
There was a problem hiding this comment.
Right - since we certainly grabbed the asset to check the clawback addr, even if the movement is 0 amount (I check, as I suppose we don't have to check a 0 clawback)
Summary
If an asset transfer moves no assets, and is not an optin or closeout transaction, do not prefetch its resources.
Test Plan
CI and manual testing.