-
Notifications
You must be signed in to change notification settings - Fork 213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
API: get and list only assets associated with the wallet #2492
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
I wonder if we should also return 404
on list assets when there are no assets associated. Currently it returns just []
.
Note to self: check this on Byron wallets once #2489 is merged.
It should return 404 when the wallet id isn't found. If the wallet is found but there are no assets, then |
Yeah, but get assets returns in
So I was thinking it would be nice consistency if list wallet returned something similar... but maybe |
To me, the 404 response is an error which means that the request path was referring to a resource that doesn't exist. |
05a337c
to
2fe1ce5
Compare
Thanks for the review and noticing the issue. bors r+ |
2491: Make estimateFee faster for large wallets r=rvl a=Anviking # Issue Number ADP-692 # Overview - [ ] <s>Unify `selectAssets` and `selectAssetsNoOutputs`</s> - Idea was to allow `estimateFee` to call both `readUTxOIndex` and `selectAssets` — simplifying the call-sites — but I didn't end up doing this now. This seems right to me still, unless I have missed something? - [x] Separate out a `readUTxOIndex` from `selectAssets` - [x] Make sure `readUTxOIndex` is called only once, when repeating the coin selection 100x to estimate fees. # Comments - Viewing diff commit-per-commit may be helpful Instead of 260s, we get 5s estimateFee time: ``` [bench-restore:Info:7] [2021-02-04 14:17:40.76 UTC] Restoring: [still restoring (4.01%)] restoration: 736.4 s Running read wallet read wallet: 4.359 s Running utxo statistics utxo statistics: 28.54 ms Running list addresses list addresses: 1.010 s Running list transactions list transactions: 25.22 s Running estimate tx fee estimate tx fee: 5.091 s [wallet:Error:60] [2021-02-04 14:18:18.44 UTC] Unexpected error following the chain: StatementAlreadyFinalized "BEGIN" [wallet:Notice:60] [2021-02-04 14:18:18.44 UTC] Destroying cursor connection at ThreadId 61 restore: StatementAlreadyFinalized "BEGIN" All results: BenchSeqResults: benchName: 90.0-percent-seq restoreTime: 736.4 s readWalletTime: 4.359 s listAddressesTime: 1.010 s listTransactionsTime: 25.22 s estimateFeesTime: 5.091 s walletOverview: number of addresses: 54498 number of transactions: 34810 = Total value of 25008452939161007 lovelace across 28241 UTxOs ... 10 18 ... 100 142 ... 1000 133 ... 10000 153 ... 100000 280 ... 1000000 1674 ... 10000000 1302 ... 100000000 1369 ... 1000000000 1768 ... 10000000000 3139 ... 100000000000 4682 ... 1000000000000 9737 ... 10000000000000 3634 ... 100000000000000 202 ... 1000000000000000 6 ... 10000000000000000 2 ... 45000000000000000 0 1,186,372,795,296 bytes allocated in the heap 187,306,453,352 bytes copied during GC 739,022,840 bytes maximum residency (626 sample(s)) 37,791,752 bytes maximum slop 704 MB total memory in use (0 MB lost due to fragmentation) Tot time (elapsed) Avg pause Max pause Gen 0 1082118 colls, 1082118 par 541.175s 136.483s 0.0001s 0.0040s Gen 1 626 colls, 625 par 58.612s 14.913s 0.0238s 0.0730s Parallel GC work balance: 10.33% (serial 0%, perfect 100%) TASKS: 17 (1 bound, 16 peak workers (16 total), using -N4) SPARKS: 0(0 converted, 0 overflowed, 0 dud, 0 GC'd, 0 fizzled) INIT time 0.002s ( 0.004s elapsed) MUT time 582.358s (638.775s elapsed) GC time 599.788s (151.396s elapsed) RP time 0.000s ( 0.000s elapsed) PROF time 0.000s ( 0.000s elapsed) EXIT time 0.000s ( 0.001s elapsed) Total time 1182.148s (790.176s elapsed) Alloc rate 2,037,187,147 bytes per MUT second Productivity 49.3% of total user, 80.8% of total elapsed Benchmark restore: FINISH Completed 2 action(s). ``` <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Jira will detect and link to this PR once created, but you can also link this PR in the description of the corresponding ticket ✓ Acknowledge any changes required to the Wiki ✓ Finally, in the PR description delete any empty sections and all text commented in <!--, so that this text does not appear in merge commit messages. --> 2492: API: get and list only assets associated with the wallet r=rvl a=rvl ### Issue Number ADP-603 / ADP-604 ### Overview - The list assets endpoint now returns all assets of the wallet, not just available assets. - The get asset endpoint checks that the asset id really is associated with the wallet. Co-authored-by: Johannes Lund <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>
Build failed (retrying...):
|
2492: API: get and list only assets associated with the wallet r=rvl a=rvl ### Issue Number ADP-603 / ADP-604 ### Overview - The list assets endpoint now returns all assets of the wallet, not just available assets. - The get asset endpoint checks that the asset id really is associated with the wallet. Co-authored-by: Rodney Lorrimar <[email protected]>
Build failed:
Is this a new one?? |
2fe1ce5
to
7ab1177
Compare
bors r+ |
2489: Add missing APIs for multi-asset r=rvl a=rvl ## Issue Number ADP-698 ## Overview - Adds assets to the responses of the coin selections endpoints. - Adds assets to the `/byron-wallets` endpoints. ## Comments 2492: API: get and list only assets associated with the wallet r=rvl a=rvl ### Issue Number ADP-603 / ADP-604 ### Overview - The list assets endpoint now returns all assets of the wallet, not just available assets. - The get asset endpoint checks that the asset id really is associated with the wallet. 2493: Let tests and cluster run in pre-mary eras again r=rvl a=rvl ### Issue Number ADP-695 ### Overview Conditionally fund the assets faucet based on era. Co-authored-by: Rodney Lorrimar <[email protected]>
Build failed (retrying...): |
2492: API: get and list only assets associated with the wallet r=rvl a=rvl ### Issue Number ADP-603 / ADP-604 ### Overview - The list assets endpoint now returns all assets of the wallet, not just available assets. - The get asset endpoint checks that the asset id really is associated with the wallet. 2493: Let tests and cluster run in pre-mary eras again r=rvl a=rvl ### Issue Number ADP-695 ### Overview Conditionally fund the assets faucet based on era. Co-authored-by: Rodney Lorrimar <[email protected]>
Build failed (retrying...):
|
2492: API: get and list only assets associated with the wallet r=rvl a=rvl ### Issue Number ADP-603 / ADP-604 ### Overview - The list assets endpoint now returns all assets of the wallet, not just available assets. - The get asset endpoint checks that the asset id really is associated with the wallet. Co-authored-by: Rodney Lorrimar <[email protected]>
Build failed:
|
bors r+ |
2489: Add missing APIs for multi-asset r=rvl a=rvl ## Issue Number ADP-698 ## Overview - Adds assets to the responses of the coin selections endpoints. - Adds assets to the `/byron-wallets` endpoints. ## Comments 2492: API: get and list only assets associated with the wallet r=rvl a=rvl ### Issue Number ADP-603 / ADP-604 ### Overview - The list assets endpoint now returns all assets of the wallet, not just available assets. - The get asset endpoint checks that the asset id really is associated with the wallet. 2493: Let tests and cluster run in pre-mary eras again r=rvl a=rvl ### Issue Number ADP-695 ### Overview Conditionally fund the assets faucet based on era. Co-authored-by: Rodney Lorrimar <[email protected]>
Build failed (retrying...): #duplicate |
2492: API: get and list only assets associated with the wallet r=rvl a=rvl ### Issue Number ADP-603 / ADP-604 ### Overview - The list assets endpoint now returns all assets of the wallet, not just available assets. - The get asset endpoint checks that the asset id really is associated with the wallet. 2493: Let tests and cluster run in pre-mary eras again r=rvl a=rvl ### Issue Number ADP-695 ### Overview Conditionally fund the assets faucet based on era. Co-authored-by: Rodney Lorrimar <[email protected]>
Build failed (retrying...): #duplicate (batch) |
2492: API: get and list only assets associated with the wallet r=rvl a=rvl ### Issue Number ADP-603 / ADP-604 ### Overview - The list assets endpoint now returns all assets of the wallet, not just available assets. - The get asset endpoint checks that the asset id really is associated with the wallet. Co-authored-by: Rodney Lorrimar <[email protected]>
Build failed:
|
bors r+ |
Build succeeded: |
Issue Number
ADP-603 / ADP-604
Overview