Expose /pending/<address> endpoint for use in Cashtab to prevent accidental double registration of aliases and track user's pending aliases to success / failure
Details
Details
- Reviewers
emack Fabien - Group Reviewers
Restricted Project - Commits
- rABC2be32d467a7a: [alias-server] Add API endpoint to get pending aliases by address
npm test
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- pending-alias-api
- Lint
Lint Errors Severity Location Code Message Error apps/alias-server/test/app.test.js:14 ESLINT no-unused-vars - Unit
No Test Coverage - Build Status
Buildable 25042 Build 49672: Build Diff alias-server-tests Build 49671: arc lint + arc unit
Event Timeline
Comment Actions
I've been looking at this from a customer (wallet dev) POV. To show the latest alias list attached to the wallet I would have to call /address to get the list of confirmed aliases and then quickly followed by /pending to get the pending list.
Is it more efficient to update the /address endpoint to return the results from /pending as well?
Especially when you think of the use cases:
- user loads Alias.js and needs to render a list of confirmed and pending aliases - one single API call would be best
- user tries to register the same alias a 2nd time and the wallet would be parsing both confirmed and pending aliases before proceeding with registration broadcast - one single API call here would also be best
- user successfully broadcasts a new alias registration which then refresh the aliases displayed on page
The /pending endpoint does have its use, if other wallets want to render the pending list separately. Just that from Cashtab's pov it would be good to just call /address to include pending aliases.
Also confirming:
- This means Cashtab no longer needs to store a pending list in localForage since everything is taken off the endpoint?
- Which also means the current interval to execute periodic pending checks is now a confirmed+pending check?
Comment Actions
This means Cashtab no longer needs to store a pending list in localForage since everything is taken off the endpoint?
Correct -- we don't want this to rely on local storage as this would make it device dependent.
Which also means the current interval to execute periodic pending checks is now a confirmed+pending check?
on load, check the pending list. if any pending, render them as such. at some fixed interval, check in on what has happened to the pending aliases. they will either become registered (appear at confirmed endpoint) or invalid (disappear from pending + some other tx with that alias is on the confirmed list). Edge case that they disappear from pending and nothing becomes confirmed, i.e. dropped from the mempool. Can also call this "invalid".
For Cashtab purposes, keep the pending list in state.
Re: adding pending to one endpoint -- yeah that makes sense, will do this.
| apps/alias-server/src/app.js | ||
|---|---|---|
| 171 ↗ | (On Diff #42232) | imo it's worth keeping a separate "just give me the pending aliases by address" endpoint as there are some use cases where Cashtab may want only this information, i.e. determining which txs need to be tracked to some kind of result. Getting all the registered aliases for this case would be overkill. Makes sense to always provide pending for /address/<address> as most dev use cases of here would need both. Can be further customized if there is a need but we don't really want the API to be a custom db query system. |
| apps/alias-server/src/app.js | ||
|---|---|---|
| 171 ↗ | (On Diff #42232) | You can just add a param to the query, like /address/<address> returns both, /address/<address>?pending=true returns pending only and /address/<address>?pending=false returns confirmed only |
| apps/alias-server/src/app.js | ||
|---|---|---|
| 171 ↗ | (On Diff #42232) | ended up being not so trivial to implement this in express. On further consideration, just simplify and do not keep a unique "pending only" feature. Cashtab will always be calling for both so will not need pending only. The need for pending-only is at this point hypothetical. Will engineer for it if the need becomes clear. |
| apps/alias-server/src/app.js | ||
|---|---|---|
| 152 ↗ | (On Diff #42252) | Did you consider using pagination here ? What happens if a user registers a ton of aliases to a single address ? |
| apps/alias-server/test/app.test.js | ||
| 137 ↗ | (On Diff #42252) | the keys in the json are strings right ? The fact that you can use a variable name is due to the software encoding/decoding ? |