-
Notifications
You must be signed in to change notification settings - Fork 371
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
BLOCK-40: add SDK functions for chainsaw #4004
Conversation
🦋 Changeset detectedLatest commit: 920ad00 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
size-limit report 📦
|
Your org requires the Graphite merge queue for merging into mainAdd the label “merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
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.
Code looks great! Just some comments on typedocs and types
packages/thirdweb/src/chainsaw/endpoints/getLatestBlockNumber.ts
Outdated
Show resolved
Hide resolved
packages/thirdweb/src/chainsaw/endpoints/getNFTsByCollection.ts
Outdated
Show resolved
Hide resolved
packages/thirdweb/src/chainsaw/endpoints/getNFTsByCollection.ts
Outdated
Show resolved
Hide resolved
47831d3
to
cca8c79
Compare
client, | ||
chain: defineChain(1) | ||
}); | ||
const events = await getEvents({ |
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.
should we build this into the existing getContractEvents that we have?
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.
Yeah we should. Right now the indexer gets events based on start and end time though and getContractEvents
does it based on block numbers. Let me ask if we can add block number filter options on the indexer, but we won't be able to fallback to RPC call when filtering by times
* }); | ||
* ``` | ||
*/ | ||
export async function getEvents( |
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.
i think if you call this inside getContractEvents() and the formats match it should do the decoding for you
type GetNFTsByCollectionParams, | ||
} from "../indexer/endpoints/getNFTsByCollection.js"; | ||
export { | ||
getNFTsByOwner, |
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.
getWalletBalance
getWalletNFTs
getWalletTokens
extensions/erc721/getOwnedNFTs
extensions/erc1155/getOwnedNFTs
type GetBlockParams, | ||
} from "../indexer/endpoints/getBlock.js"; | ||
export { | ||
getLatestBlockNumber, |
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.
dont expose block functions
type GetLatestBlockNumberParams, | ||
} from "../indexer/endpoints/getLatestBlockNumber.js"; | ||
export { | ||
getNFTsByCollection, |
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.
should just be injected inside
extensions/erc721/getNFTs
extensions/erc1155/getNFTs
type GetNFTsByOwnerParams, | ||
} from "../indexer/endpoints/getNFTsByOwner.js"; | ||
export { | ||
getContractTransactions, |
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.
this one is good as is
* const block = await getContractTransactions({ | ||
* contract, | ||
* pageSize: 20, | ||
* page: 1 |
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.
change to start and count
Problem solved
Added the following chainsaw methods to the SDK
NB! Will continue the comments & changes from this PR here
PR-Codex overview
This PR adds chain ID and contract address to NFT retrieval functions, introduces indexer endpoints, and updates domain URLs.
Detailed summary
chainId
andcontractAddress
to NFT retrieval functionschainId
andcontractAddress
to NFT parsing options