Skip to content

ledger: fix lookupAssetResources/lookupApplicationResources delta-merge bugs#6577

Merged
algorandskiy merged 2 commits intoalgorand:masterfrom
cce:6559-tests
Mar 6, 2026
Merged

ledger: fix lookupAssetResources/lookupApplicationResources delta-merge bugs#6577
algorandskiy merged 2 commits intoalgorand:masterfrom
cce:6559-tests

Conversation

@cce
Copy link
Copy Markdown
Contributor

@cce cce commented Mar 6, 2026

Summary

Fixes three bugs in lookupAssetResources and lookupApplicationResources introduced by #6559, with tests.

Phantom zero-value holding/local-state from DB rows

The DB merge fallback (acctupdates.go:1300, 1446) called GetAssetHolding() / GetAppLocalState() without checking pd.Data.IsHolding() first. The DB knows whether a row has a holding via ResourceFlags, but the code ignored the flags and unconditionally materialized a zero-value struct. This kept rows alive through the (AssetHolding != nil || AssetParams != nil) filter when they should have been excluded. The correct pattern is already used in AccountResource() at trackerdb/data.go:272.

Fixed by guarding both with pd.Data.IsHolding().

numDeltaDeleted under-count

The over-request counter only checked Holding.Deleted (assets) or State.Deleted (apps). A delta that deletes only params on a row with no holding/local-state (e.g., an app creator who never opted in) was not counted, so the DB page was too small and the result page came up short.

Fixed by counting either deletion: Holding.Deleted || Params.Deleted (and same for apps).

Empty-page StaleDatabaseRoundError

The SQL query for limited resources uses INNER JOINs, so when the address has no matching resources, zero rows are returned and the round stays at its zero value. The code then fell through to resourceDbRound < currentDBRound and returned a StaleDatabaseRoundError.

Fixed by accepting the zero-row case: resourceDbRound == currentDBRound || (len(persistedResources) == 0 && resourceDbRound == 0).

Test Plan

Three new tests cover the bugs:

  • TestLookupAppResourcesParamsOnlyDeletion: params-only DB row + delta deletion
  • TestLookupAssetResourcesEmptyPageDoesNotError: empty asset lookup
  • TestLookupApplicationResourcesEmptyPageDoesNotError: empty app lookup
go test -v -run 'TestLookup.*Resources' ./ledger/ -count=1

@cce cce added the Bug-Fix label Mar 6, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 47.90%. Comparing base (25e0d3a) to head (9564ec1).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

❗ There is a different number of reports uploaded between BASE (25e0d3a) and HEAD (9564ec1). Click for more details.

HEAD has 4 uploads less than BASE
Flag BASE (25e0d3a) HEAD (9564ec1)
full_coverage 4 0
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #6577       +/-   ##
===========================================
- Coverage   63.02%   47.90%   -15.12%     
===========================================
  Files         486      646      +160     
  Lines       68126    88226    +20100     
===========================================
- Hits        42935    42268      -667     
- Misses      21404    43182    +21778     
+ Partials     3787     2776     -1011     
Flag Coverage Δ
full_coverage ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cce cce changed the title ledger: add failing test for lookupApplicationResources phantom local state ledger: fix lookupAssetResources/lookupApplicationResources delta-merge bugs Mar 6, 2026
Comment thread ledger/acctdeltas_test.go
@algorandskiy algorandskiy merged commit 7b03566 into algorand:master Mar 6, 2026
40 checks passed
@cce cce deleted the 6559-tests branch March 7, 2026 01:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants