ledger: replace Ledger.LookupResource with Lookup{Application,Asset} #3708
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3708 +/- ##
==========================================
+ Coverage 49.56% 49.57% +0.01%
==========================================
Files 391 391
Lines 68535 68560 +25
==========================================
+ Hits 33970 33991 +21
- Misses 30829 30832 +3
- Partials 3736 3737 +1
Continue to review full report at Codecov.
|
| if task.creatableType == basics.AppCreatable { | ||
| var appResource ledgercore.AppResource | ||
| appResource, err = p.ledger.LookupApplication(p.rnd, *task.address, basics.AppIndex(task.creatableIndex)) | ||
| resource.AppParams = appResource.AppParams |
There was a problem hiding this comment.
It looks that with your change, we should also change the loadedTransactionGroup.resources into the two explicit lists of assets and applications. This could be done later on, though.
There was a problem hiding this comment.
yes, I wasn't sure if that was in scope or whether it was OK for behind-the-scenes implementations further away from the external-facing ledger interfaces to deal with resources. But yes it would mean splitting loadAccountsAddResourceTask into two functions/tasks and maybe could be put off for when someone works on the harness to catch differences between calls to apply.Balances from a suite of test transactions and the resources that are queued for prefetching.
| } | ||
|
|
||
| // lookupResource loads a resource that matches the request parameters from the accounts update | ||
| func (l *Ledger) lookupResource(rnd basics.Round, addr basics.Address, aidx basics.CreatableIndex, ctype basics.CreatableType) (ledgercore.AccountResource, error) { | ||
| l.trackerMu.RLock() | ||
| defer l.trackerMu.RUnlock() | ||
|
|
There was a problem hiding this comment.
what rewards is this talking about?
There was a problem hiding this comment.
you're correct, the comment is wrong and need to be removed. ( regardless of this change )
Summary
This replaces the various Ledger interfaces'
LookupResourcemethod with two more specificLookupApplicationandLookupAssetmethods. Following up on code review feedback from #3652.Test Plan
Existing tests should pass, including the ones that implement their own mock ledger.