Comments (4)
To avoid this issue we could change API functions with pagination to return an async iterator instead of Promise.
Would the wallet do something like ordering all utxos and then have the returned generator remember the last returned one and query the wallet for the next one in whatever arbitrary ordering they chose yielding that until it's there are none? I'm not primarily a javascript dev (more C++/Rust) and haven't worked with this feature before so I just want to be sure this would be easily doable and solve the issues with the current API.
Is the dApp supposed to try to call the method with {limit: Number.POSITIVE_INFINITY} to figure out max size? I think it's an odd API and if we keep the concept of pagination there should be a way to get the limit without an error.
The idea was they would pick a limit
based on their own need like 10 or 50 or whatever and repeatedly call it with increasing page
numbers until PaginateError
is returned. I guess if you wanted to know how many pages there would be you could use infinity. To be honest I'm not really familiar with typical paging APIs so I had hoped someone would point out better ways during the original PR review if possible. I guess the above generator approach wouldn't let you figure out how many it would return in total either though?
I initially thought that TransactionUnspentOutput wasn't cbor, the definition could use better consistency: I think we should either define all types as cbor<[type]> or list all type definitions under Data Types.
Do you mean including all the types that are defined via cbor<T>
explicitly? The problem with this is that they are just composed of other types and including those too would be a lot of types. Plus I think it's best to refer to directly to the binary spec. All other types are listed under Data Types
so I don't think it's a problem.
Does it include rewards and deposit? Need to make this explicit, as using these funds requires certificates in the transaction.
This was discussed in #88 here but we never finalized any changes. Perhaps it's time to continue that discussion that was started there and get to a final solution. @KtorZ @SebastienGllmt @alessandrokonrad
There will be different types of wallets. Some might be single address and not have any unused addresses. Given this API, dApp will probably expect to always get some unused addresses, which might not be the case. The dApp itself can figure out if it was used or not. I suggest to consolidate these to api.getAddresses(paginate: Paginate = undefined): Promise<cbor
[]>.
Do we expect the dApp to filter them all to see if they've ever been used on-chain before? I think people would be using the two endpoints (used and unused) for very different purposes. We could change the API description to mention that they shouldn't expect there to always be addresses if that would help. We should see what others think though.
from cips.
Would the wallet do something like ordering all utxos
I think we should be thinking in terms of sets of utxo (unordered).
and then have the returned generator remember the last returned one and query the wallet for the next one in whatever arbitrary ordering they chose yielding that until it's there are none?
I think we shouldn't assume any specific wallet implementation or data source. It could already have all utxo in memory, could be fetching it from some server, or loading from indexeddb. Generator implementation would depend on wallet internals, but from API user point of view it would be a generator of the most recent snapshot of the utxo set.
I'm not primarily a javascript dev (more C++/Rust) and haven't worked with this feature before so I just want to be sure this would be easily doable and solve the issues with the current API.
Not yielding duplicates should be trivial to implement regardless of where the wallet is sourcing the utxo (checking by tx id + index). The wallets can also not yield a utxo once it's spent, even if it's spent after returning the generator. There might still be a situation where a utxo that was yielded is spent before the generator completes. There could be an error for that.
For this specific function of api.getUtxos
, I actually suggest just api.getUtxos(amount: cbor<value> = undefined): Promise<TransactionUnspentOutput[]>
. What was the idea behind pagination for this one in the first place?
The idea was they would pick a limit based on their own need like 10 or 50 or whatever and repeatedly call it with increasing page numbers until PaginateError is returned. I guess if you wanted to know how many pages there would be you could use infinity.
I thought the PaginateError
error was for a too large page size requested. I don't think there's much value in having an error for when there are no more pages, as it can be inferred from an empty array result. I actually suggest to just remove this error.
To be honest I'm not really familiar with typical paging APIs so I had hoped someone would point out better ways during the original PR review if possible.
A common pattern is to return something like {items: Item[], numTotalItems: number}
.
I guess the above generator approach wouldn't let you figure out how many it would return in total either though?
Yes, you wouldn't know the total number of items, but you can stop taking them when you got enough.
Do you mean including all the types that are defined via cbor explicitly?
That's one option, but I suggest getUtxos
is declared as: api.getUtxos(amount: cbor<value> = undefined, paginate: Paginate = undefined): Promise<cbor<transaction_unspent_output>[] | undefined>
, not using the TransactionUnspentOutput
type declared above for consistency (it's the only type declared this way).
This was discussed in #88 here but we never finalized any changes. Perhaps it's time to continue that discussion that was started there and get to a final solution. @KtorZ @SebastienGllmt @alessandrokonrad
The discussion seems to be about total vs available balance/utxo. I think it's ok for the connector to always return available balance/utxo (would be great to document it though). My question was about what this balance consists of. I suggest returning something like that: { utxo: cbor<value>; rewards: Lovelace; keyDeposit: Lovelace; poolDeposit: Loverlace; }
Do we expect the dApp to filter them all to see if they've ever been used on-chain before? I think people would be using the two endpoints (used and unused) for very different purposes. We could change the API description to mention that they shouldn't expect there to always be addresses if that would help. We should see what others think though.
Yes I was thinking the dApp would filter them, but now that you mention very different purposes I no longer think that consolidating them is a good idea. Changing the description would be a good improvement.
from cips.
Thanks for all the feedback.
I think we should be thinking in terms of sets of utxo (unordered).
I wasn't saying it should be in the spec, I was just thinking of a possible way to implement it that would solve both issues with the previous approach, those being that you might skip utxos if the set changed mid-iteration (e.g. you are on page 5 and something in pages 1-4 was spent so now everything shifts and you might miss a utxo) and that maybe new utxos will be skipped in a similar way. Having some kind of ordering was just the first idea that came to mind for me in order to implement it in a way that solves both of those issues - I didn't mean to have it sound like I was mandating it as the only way.
For this specific function of api.getUtxos, I actually suggest just api.getUtxos(amount: cbor = undefined): Promise<TransactionUnspentOutput[]>. What was the idea behind pagination for this one in the first place?
We might have been overly conservative of which endpoints might need pagination since it is optional functionality anyway. If you don't pass in a Paginate
instance (it default to undefined) then you get back everything.
That's one option, but I suggest getUtxos is declared as: api.getUtxos(amount: cbor = undefined, paginate: Paginate = undefined): Promise<cbor<transaction_unspent_output>[] | undefined>, not using the TransactionUnspentOutput type declared above for consistency (it's the only type declared this way).
The problem is that transaction_unspent_output
isn't in the binary spec. It's just a convenience type that we've made up since using just the spec's input type doesn't give you the value which makes it useless for figuring out input selection without consulting the ledger. Or are you talking about adding it in the cbor<T>
header as a raw cddl definition and then referring to it with cbor<transaction_unspent_output>
directly everywhere else?
from cips.
Thanks for all the feedback.
Thanks for considering my suggestions.
Having some kind of ordering was just the first idea that came to mind for me in order to implement it in a way that solves both of those issues
If we think about the issues separately, duplicate items issue is really just some complexity that needs to be handled either by the wallets or by the dapps. Both can do it, but I think this API should aim to make dapp development easy.
I have an alternative suggestion. Instead of optional pagination argument, how about a separate API method that you would use like this:
const pageSize = 10;
const { numPages, getPage } = await api.paginateUsedAddresses(pageSize);
for (let page=0; page<numPages; page++) {
const addressesPage = await getPage(page); // an array of addresses with length up to pageSize
}
We might have been overly conservative of which endpoints might need pagination since it is optional functionality anyway.
I think amount
argument is sufficient. I'm wondering if I'm missing some use case.
The problem is that transaction_unspent_output isn't in the binary spec.
Oh, I see. I misunderstood the purpose of that type def. Thanks for an explanation.
from cips.
Related Issues (20)
- CIP-1854 | Adjust preamble and structure w.r.t CIP-0001
- CIP-1855 | Adjust preamble and structure w.r.t CIP-0001
- Add References section to CIPs/CPSs HOT 10
- CIP-100: Make witnessing optional + add CIP-08 message signing HOT 5
- CIP-9999 | How to add Copyright to CPSs HOT 4
- CIP-0069: alternative approach HOT 10
- CIP-0072: Suggestions for improving dApp registration identity verification HOT 4
- CIP-100 | Canonical example is wrong HOT 3
- CIP 100 | Provide directions on how to create signatures for the body without circular dependencies. HOT 19
- Wiki: Initial topics + points to cover HOT 8
- CIP-0058: integers vs. byteStrings for cryptographic applications HOT 32
- Rendering errors on cips.cardano.org. HOT 2
- CIP-0013: Current state of integration and further advancements HOT 13
- CIP-0010 | Add schema links HOT 2
- Voter metadata sign the initial document HOT 1
- Tracking protocol parameters definitions HOT 3
- Indexing CIPs Repo for Core `Waiting for Implementation` proposals HOT 1
- CIP-0021 Transformation Breaks Script Integrity HOT 11
- CIP PR queue overhaul, state vocabulary & tagging HOT 10
- CIP-105 / CIP-1852: suggested HD paths incompatible with security policy of Ledger HWW for Conway roles HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from cips.