Feature: Add support for unlimited assets. #900

cce · 2022-02-24T20:46:29Z

Summary

Companion updates for algorand/go-algorand#3652

Adds new REST endpoints and "exclude" parameter for account information endpoint, similar to algorand/go-algorand#3542.

Update endpoint: GET /v2/accounts/{address}

Supports new query parameter “exclude”, which can be a combination of all, created-apps, created-assets, apps-local-state, assets, none
Removes empty “creator” string from AssetHolding type — was not implemented (removed associated TODO)

Update endpoint: GET /v2/assets/{asset-id}/balances

Remove "round" query parameter — I noticed support is not implemented, and the parameter was being ignored

New endpoints:
GET /v2/accounts/{address}/created-apps
GET /v2/accounts/{address}/created-assets
GET /v2/accounts/{address}/assets
GET /v2/accounts/{address}/apps-local-state

Supported query parameters: asset-id / application-id (look up a single asset), include-all (include deleted), limit, next (for pagination)

Test Plan

New tests added, existing tests should pass.

* Ledger refactoring changes. * Move some of the preloading logic to accounting/. * Use setResourcesReq(). * Some renaming. * Add comments to ledger_for_evaluator.LookupResources(). * Encode state proof id.

#896) * backwards-compatible JSON encodings for account data * update encoding test for new field names * CR fixes * CR fix b

Adds new REST endpoints and "exclude" parameter for account information endpoint, similar to algorand/go-algorand#3542. Relies on #870 Update endpoint: GET /v2/accounts/{address} Supports new query parameter “exclude”, which can be a combination of all, created-apps, created-assets, apps-local-state, assets Removes empty “creator” string from AssetHolding type — was not implemented (removed associated TODO) Update endpoint: GET /v2/assets/{asset-id}/balances Remove "round" query parameter — I noticed support is not implemented, and the parameter was being ignored New endpoints: GET /v2/accounts/{address}/created-apps GET /v2/accounts/{address}/created-assets GET /v2/accounts/{address}/assets GET /v2/accounts/{address}/apps-local-state Supported query parameters: asset-id / application-id (look up a single asset), include-all (include deleted), limit, next (for pagination)

…assets

…#901) * REST API: count include-all results when checking max resources limit * update TestValidate for removed round param * move countOnly to function argument rather than polluting idb interface

codecov-commenter · 2022-02-25T19:02:29Z

Codecov Report

Merging #900 (2b82173) into develop (def3543) will increase coverage by 0.98%.
The diff coverage is 76.15%.

@@             Coverage Diff             @@
##           develop     #900      +/-   ##
===========================================
+ Coverage    57.94%   58.93%   +0.98%     
===========================================
  Files           40       42       +2     
  Lines         4713     5720    +1007     
===========================================
+ Hits          2731     3371     +640     
- Misses        1646     1902     +256     
- Partials       336      447     +111

Impacted Files	Coverage Δ
api/generated/common/routes.go	`33.33% <ø> (ø)`
idb/dummy/dummy.go	`0.00% <0.00%> (ø)`
idb/idb.go	`71.42% <0.00%> (+12.89%)`	⬆️
idb/postgres/internal/encoding/types.go	`66.66% <ø> (ø)`
api/generated/v2/routes.go	`18.92% <55.78%> (ø)`
idb/postgres/postgres_migrations.go	`43.11% <60.00%> (+8.01%)`	⬆️
...gres/internal/migrations/convert_account_data/m.go	`63.15% <63.15%> (ø)`
api/handlers.go	`74.31% <78.18%> (+5.03%)`	⬆️
idb/postgres/postgres.go	`60.24% <78.91%> (+10.88%)`	⬆️
accounting/eval_preload.go	`81.13% <81.13%> (ø)`
... and 44 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update def3543...2b82173. Read the comment docs.

algonautshant · 2022-02-25T20:29:16Z

api/indexer.oas2.json

@@ -455,9 +651,6 @@
          {
            "$ref": "#/parameters/next"
          },
-          {
-            "$ref": "#/parameters/round"


Will this impact sdk backwards compatibility? Is this removed for both algod and indexer? Please comment.

Yes, anyone who was providing this argument with an SDK will have a compilation error, and anyone who was using without the SDK will receive a 400 (since the backend will not know what "round" is for). After talking with @winder and @algoanne I believe this is preferable to the current behavior — where the round parameter is being silently ignored, the latest round's data is returned, but the caller thinks they are getting a historical value from an old round.

algonautshant · 2022-02-25T20:29:31Z

api/indexer.oas2.json

-        "creator": {
-          "description": "Address that created this asset. This is the address where the parameters for this asset can be found, and also the address where unwanted asset units can be sent in the worst case.",
-          "type": "string"
-        },


Will this impact sdk backwards compatibility? Please comment.

Yes, similarly, if anyone was expecting this value using the SDK it will no longer compile. However, this value was always being returned as an empty string and was never implemented. So anyone who has ever written code that retrieved this field should have already found it empty and useless, and likely was not depending on it.

mxmauro · 2022-03-01T19:45:05Z

I would like to suggest a change in the new implementation of GET /v2/accounts/{address}: Not to return an error if the number of apps/assets is high and instead add a message/flag to the response indicating that there are more items to instruct the user to call the new endpoints. May be an extra flag in the query.

Reason:

Assuming most of users won't have a lot of opted-in assets/apps, any UI like wallets would be able to fill the whole information with just one call to the backend.

If an error is returned, the UI must call the endpoint with exclude all and then the new apis (one for each type of information they want to retrieve) in order to get the first page of assets/apps they use.

cce · 2022-03-01T20:30:08Z

We were concerned that for backwards compatibility reasons, giving callers a "trimmed down" account for some cases, but a full one in others, would leave them with the false impression that the account was empty, especially if they were using an older SDK.

Since most users won't have a lot of opted-in assets, like you said the error case is rare, but clear. Optimizing the number of calls for the rare error case wasn't a priority when we talked this through.

One hope we had, and considered making a requirement, was that developers would adopt exclude=all everywhere, and then only pull information about assets/apps when truly necessary. This would be the most error-proof / future-proof way to look up account information and saves the indexer a lot of unnecessary work.

For those that choose not to adopt exclude=all for their initial lookup (e.g. to check counts of opted-in assets/apps) and are willing to "risk" an optimistic exclude=none, do you think it is too onerous to have them implement the 400, then re-request flow? It is just one more call, for what should be a rare occurrence.

cce · 2022-03-01T20:44:41Z

Oh I re-read. So an extra flag that says "give me a trimmed version if it goes over the limit." I guess that is "exclude=sometimes" :P or some better term.. that would address the backwards compatibility concern.

…ssets

…assets

Makefile

api/handlers.go

winder

Aside from the merge conflict, this still looks good!

…assets

* use ErrorResponse instead of AccountsErrorResponse * update for ErrorResponse format in tests * remove unnecessary pointers from preparing ErrorResponse

onetechnical and others added 8 commits February 1, 2022 09:50

Merge branch 'release/2.8.3'

bd40522

Merge branch 'release/2.8.4'

4997285

Ledger refactoring changes (#870)

ae96643

* Ledger refactoring changes. * Move some of the preloading logic to accounting/. * Use setResourcesReq(). * Some renaming. * Add comments to ledger_for_evaluator.LookupResources(). * Encode state proof id.

unlimited assets: backwards-compatible JSON encodings for account data (

76faddd

#896) * backwards-compatible JSON encodings for account data * update encoding test for new field names * CR fixes * CR fix b

Merge remote-tracking branch 'origin/develop' into feature/unlimited-…

dadae50

…assets

REST API: count include-all results when checking max resources limit (…

8444586

…#901) * REST API: count include-all results when checking max resources limit * update TestValidate for removed round param * move countOnly to function argument rather than polluting idb interface

add "none" to exclude enum

20c5ade

rename MaxAccountNestedObjects => MaxAPIResourcesPerAccount

ebc44d5

algonautshant reviewed Feb 25, 2022

View reviewed changes

cce added 3 commits February 25, 2022 16:06

update docs for MaxAccountNestedObjects => MaxAPIResourcesPerAccount

b3ebf76

remove "creator" from required since it has been removed from response

0a20fa2

achieve parity

12aaa71

cce mentioned this pull request Feb 28, 2022

Algod : merge unlimited assets to master algorand/go-algorand#3652

Merged

4 tasks

algobarb and others added 2 commits March 2, 2022 11:57

Merge branch 'release/2.9.0'

6bc3394

Merge remote-tracking branch 'origin/master' into feature/unlimited-a…

6fdb088

…ssets

cce requested a review from winder March 8, 2022 21:52

cce added 2 commits March 8, 2022 17:11

Merge remote-tracking branch 'origin/develop' into feature/unlimited-…

ff54cba

…assets

add DisabledMapConfig to handles_e2e_test defaultOpts

333fedf

tsachiherman reviewed Mar 9, 2022

View reviewed changes

Makefile Show resolved Hide resolved

tsachiherman reviewed Mar 9, 2022

View reviewed changes

api/handlers.go Show resolved Hide resolved

winder approved these changes Mar 9, 2022

View reviewed changes

Merge remote-tracking branch 'origin/develop' into feature/unlimited-…

6402d35

…assets

tsachiherman approved these changes Mar 9, 2022

View reviewed changes

cce and others added 2 commits March 10, 2022 10:14

REST API: use ErrorResponse instead of AccountsErrorResponse (#916)

482097a

* use ErrorResponse instead of AccountsErrorResponse * update for ErrorResponse format in tests * remove unnecessary pointers from preparing ErrorResponse

Merge branch 'develop' into feature/unlimited-assets

2b82173

winder merged commit 31eec10 into develop Mar 10, 2022

winder changed the title ~~Updates for unlimited assets~~ Feature: Add support for unlimited assets. Mar 10, 2022

winder deleted the feature/unlimited-assets branch March 10, 2022 17:56

algobarb mentioned this pull request Mar 10, 2022

DO NOT MERGE: indexer 2.10.0 #923

Merged

algobarb mentioned this pull request Apr 28, 2022

FOR REVIEW ONLY - go-algorand-sdk v1.15.0 algorand/go-algorand-sdk#313

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Add support for unlimited assets. #900

Feature: Add support for unlimited assets. #900

cce commented Feb 24, 2022 •

edited

Loading

codecov-commenter commented Feb 25, 2022 •

edited

Loading

algonautshant Feb 25, 2022

cce Feb 28, 2022

algonautshant Feb 25, 2022

cce Feb 28, 2022

mxmauro commented Mar 1, 2022

cce commented Mar 1, 2022 •

edited

Loading

cce commented Mar 1, 2022

winder left a comment

Feature: Add support for unlimited assets. #900

Feature: Add support for unlimited assets. #900

Conversation

cce commented Feb 24, 2022 • edited Loading

Summary

Test Plan

codecov-commenter commented Feb 25, 2022 • edited Loading

Codecov Report

algonautshant Feb 25, 2022

Choose a reason for hiding this comment

cce Feb 28, 2022

Choose a reason for hiding this comment

algonautshant Feb 25, 2022

Choose a reason for hiding this comment

cce Feb 28, 2022

Choose a reason for hiding this comment

mxmauro commented Mar 1, 2022

cce commented Mar 1, 2022 • edited Loading

cce commented Mar 1, 2022

winder left a comment

Choose a reason for hiding this comment

cce commented Feb 24, 2022 •

edited

Loading

codecov-commenter commented Feb 25, 2022 •

edited

Loading

cce commented Mar 1, 2022 •

edited

Loading