neume-network / strategies

Indexing strategies for the neume network.
GNU General Public License v3.0
11 stars 7 forks source link

feat/add-call-tokenuri #315

Closed Kunkka0822 closed 1 year ago

TimDaub commented 1 year ago

@Kunkka0822 why can't we be using https://github.com/neume-network/strategies/tree/main/src/strategy-factories/call-tokenuri ?

Kunkka0822 commented 1 year ago

@Kunkka0822 why can't we be using https://github.com/neume-network/strategies/tree/main/src/strategy-factories/call-tokenuri ?

As you can see, we need more functions and in my opinion, these are not designed to be extensible.

il3ven commented 1 year ago

Extractor is trying to fetch all tokenURIs for all tokenIDs. Good.

It relies on the nextTokenId function to get all tokenIds. If the nextTokenId is 100, the extractor will fetch all tokenURIs from 1 to 100.

My preferred way to get all tokenIds would be call-block-logs. We can track the Transfer event.

@Kunkka0822 also tries to transform the output from extractor using a write function. Such transforms can be done in the transformer file.

The output of the write function strips all the essential metadata such as contract address and block number. This information will be useful later. The final NFT output should be according to our defined schema and it has fields such as createdAt.

I didn't understand the need of the current transformer. Instead of writing two tokenURIs such as ar://b2dea../1 and ar://b2dea../2. It only writes ar://b2dea../[1-2]. It minimises the data to be written but I don't think it is enough reason to implement it. Such notation will increase our logic in subsequent sound-protocol strategies where we have to decode this first. An example of subsequent sound-protocol strategy is sound-protocol-get-tokenuri which fetches the ar:// URI.


Actionable advice from my side:

il3ven commented 1 year ago

I talked with @Kunkka0822 over Discord and explained my above comment. We have decided to:

TimDaub commented 1 year ago

ok good makes sense