-
Notifications
You must be signed in to change notification settings - Fork 23
API 54/search types #1679
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 54/search types #1679
Conversation
8c0b47e to
7658aa3
Compare
Deploying beaconchain with
|
| Latest commit: |
48cef6b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9dd836e4.beaconchain.pages.dev |
| Branch Preview URL: | https://api-54-search-types.beaconchain.pages.dev |
ccb9edb to
ca111b9
Compare
remoterami
left a comment
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.
left some comments, ptal
frontend/types/api/search.ts
Outdated
| } | ||
| export interface SearchToken { | ||
| address: Address; | ||
| token: 'erc20' | 'erc721' | 'erc1155'; // currently only erc20 tokens can be found |
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.
question: can tokens be found by name too or just by address?
and if i get this right, we cannot find e.g. USDC for now?
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.
USDC is an ERC20 token, so you should be able to find this.
But only via address yes, searching via name depends on what name should refer to. If it's the contract name, it's likely not a good idea as anyone can name their contract however they want, so by searching for USDC you would probably get a list of 10k scam contracts and 1 legit one.
If name refers to some internal labeling system (which we've had ideas for floating around for a while) it would work in theory, but at least I don't see how that would be working by looking at the code, and it needs to be actively maintained.
If it refers to something else, it should be specified.
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.
thanks, today I learnt.
I meant searching for the token name (like Tether, USDC, etc).
remoterami
left a comment
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.
left a small comment, otherwise utack
| IsContract: true, | ||
| Label: foundAddress.Name, | ||
| }, | ||
| Token: foundAddress.Token, |
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.
nitpick: The bigtable func will return ERC20 while the API specifies the tstype to be erc20, should streamline the capitalization
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.
fixed, ptal
2017455 to
48cef6b
Compare
searchTypeMap