raribleIngestor #24

s29papi · 2024-07-05T21:35:46Z

Ingestor for Rarible

s29papi · 2024-07-10T17:27:30Z

@chrismaddern Could you check this out

chrismaddern · 2024-07-16T01:44:39Z

Thank you for this submission!! 🥳 🙏

We've gotten a little behind on reviewing these, but will be spending some time over the next 48 hours, so look out for some feedback soon! Thank you! 🙏

chrismaddern · 2024-07-16T19:24:51Z

src/ingestors/rarible/index.ts

+        if (unitMintTokenAddress == '0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE' || unitMintTokenAddress == '0x4200000000000000000000000000000000000006') {
+            totalPriceWei = unitMintPrice;
+        } else {
+            totalPriceWei = await getRaribleMintPriceInEth(unitMintPrice, unitMintTokenAddress);


Is this common?

FWIW we don't currently support mints for mobile minting in non-native currencies (signer wallets have ETH on Ethereum network chains, and SOL on Solana)

We could exclude non ether mints. I observed at the time of making this that there were a handful of non ether mints mostly in degen.

That would be fine for now. Love the idea of supporting them in the future, but the Floor signing fleet doesn't know how to pay in Degen yet anyway 🤣

chrismaddern · 2024-07-16T19:27:20Z

src/ingestors/rarible/offchain-metadata.ts

+): Promise<string | undefined> => {
+    const apiKey = process.env.RARIBLE_API_KEY;
+    if (!apiKey) {
+        throw new Error('RARIBLE_API_KEY is not defined in the environment variables');


Deployment note -- requires RARIBLE_API_KEY to be set.

Confirming availability of this key.

chrismaddern · 2024-07-16T19:45:02Z

src/ingestors/rarible/onchain-metadata.ts

+    unitMintTokenAddress: string,
+  ): Promise<any> => {
+    const apiKey = process.env.OX_API_KEY;
+    if (!apiKey) {


Note that we likely won't include a 0x key in the project

Is it guaranteed that the result of a conversion on 0x will yield an amount that will be accepted by the contract for the mint here?

This is cool and interesting, but as I mentioned above, likely wouldn't be supported by mobile minting today anyway.

(I'm assuming it would require sending that payment token with the mint?)

Sticking to only ether based mints, removes the need for an OX api key as this is used for conversion.

chrismaddern · 2024-07-16T19:49:53Z

src/ingestors/rarible/index.ts

+       mintBuilder.setMintInstructions({
+            chainId,
+            contractAddress,
+            contractMethod: 'claim',


I don't see this method in the ABI?

chrismaddern · 2024-07-16T19:51:30Z

Thanks for taking the time to share this!

I tried it on a new BASE mint using:

yarn run dry-run rarible url https://rarible.com/collection/base/0x968ca01b5C32234F4d6Bfd44fF079BE14789bA10/drops

I got an RPC error output:

Any idea if I'm doing something wrong if there's an edge case in the ingestor?

(I added a Rarible API key to the environment on the test machine)

s29papi · 2024-07-16T20:41:38Z

two different mints, two different contract methods:

mint you are testing: https://basescan.org/address/0x968ca01b5C32234F4d6Bfd44fF079BE14789bA10#readContract

mint type my test assumptions were based of: https://basescan.org/address/0xcc3725d7e946d512a6C439E716C7C5A41cCce182#readContract

The error originates from the contract method 'sharedMetadata' which is missing in the new mint type.

The way around this would be to get the metadata stored on ipfs, and to see if it works for both. It should.

chrismaddern · 2024-07-19T12:50:20Z

Amazing, that sounds smart.

FYI — I've updated the main branch on floor/mobile-minting to add simulation to the test suite for Base.

If you:

Merge floor/mobile-minting main into your branch
Add SIMULATE_DURING_TESTS=true to your .env
Use the basicIngestorTests macro to your test suite (example here)

Then... running it should show you which created mints will actually execute onchain 🥳

One thing I noticed (beyond the fact it will only support Base, not Degen) is that the encoded parameters in the contractParams have been truncated to exponent form - e.g. 1e21, vs. a bigint string form "10000000000...". Unless the contract is explicitly expecting that, I expect that will fail.

Let me know if you'd like to land this, we're holding the bounty spot for this one if you do.

s29papi · 2024-07-22T15:11:50Z

Hey @chrismaddern, Could the data type for the successUrls be changed from string[] ---> [{url: "wyz.com", block: "0x452395"}, ...]. or block be added to type MintContractOptions or it could be the key while still retaining passing various block.

Currently the simulations blocks is a mapping of chainId to BlockHeight this is not ideal for me as each contract or url has a period of time where minting is valid and after with simulating a transaction fails, and the issue in this case being the key (chainId).

Any change that includes adding the block to the successUrls or key for blocks, it would be feasible to pass various blocks while iterating:

const result = await simulateEVMTransactionWithAlchemy(mintInstructions, simulationBlocks[mintInstructions.chainId]);

chrismaddern · 2024-07-22T19:56:17Z

Hi @s29papi if you're confident that this is an overlapping blockhash issue vs. a code generation issue then I can take a look at updating the format.

I was hoping that for now we'd be able to find a couple test URLs that were available in the same time window to begin, but it does seem likely we'd need to iterate on this eventually anyway, so.....

s29papi · 2024-07-22T20:05:03Z

If the direction for now is just mints within the same mint period, it's not a problem I would adjust to that.

s29papi · 2024-07-23T01:58:30Z

This should be up for review. Regarding the initial issue I raised, I provided an array of objects that include both block height and URL, along with an index that can be manually changed to test all mints with their respective block heights.

chrismaddern · 2024-07-23T13:57:43Z

src/ingestors/rarible/index.ts

+        mintBuilder.setName(name).setDescription(description).setFeaturedImageUrl(imageURI);
+        mintBuilder.setMintOutputContract({ chainId: chainId, address: contractAddress });
+        mintBuilder.setCreator({
+                name: 'not available',


This will show up in the UI which we don't want.

It does look like rarible.org don't include this in the public API with key, but rarible.com do provide it in their API which appears accessible

curl 'https://rarible.com/marketplace/api/v4/profiles/list' \ -H 'accept: */*' \ -H 'content-type: application/json' \ --data-raw '["0xbab964a06fbd884b0ad0efccb86f5931248858cc"]'

This has the name and other details which would make this much better! 🙂

Hey, thanks. Been in and out of their telegram chat and couldn't get this endpoint.

chrismaddern · 2024-07-23T13:59:13Z

This should be up for review. Regarding the initial issue I raised, I provided an array of objects that include both block height and URL, along with an index that can be manually changed to test all mints with their respective block heights.

If there aren't matching mints that we can use in the same time window, I'll go update the tests and commit to main an update to allow specifying a <URL/Contract, Blockhash> pair

s29papi · 2024-07-23T16:11:33Z

This should be up for review. Regarding the initial issue I raised, I provided an array of objects that include both block height and URL, along with an index that can be manually changed to test all mints with their respective block heights.

If there aren't matching mints that we can use in the same time window, I'll go update the tests and commit to main an update to allow specifying a <URL/Contract, Blockhash> pair

Alright, I would update my test file after you've made the changes.

…inting into raribleIngestor

s29papi · 2024-07-26T23:04:00Z

@chrismaddern, Took timeout to dig more into this, oops! my apologies, I was able to Find two mints which share same range for active claims within a particular block height. Would two be enough ? Although I had tested 6 mints individually which all passed at with their respective block height.

s29papi · 2024-08-12T03:04:26Z

Hey @chrismaddern I would like to know what's left to land this I have multiple tests passed, but this is still marked on final approach

chrismaddern · 2024-08-27T10:53:22Z

Hi @s29papi sorry for the delay!

I was hoping to be able to give you a quick thumbsup, but it seems one of the tests is now not passing?

Can you get that passing and then we can get it merge.

A quick spotcheck with Rarible mint looks great!

yarn dry-run rarible url https://rarible.com/collection/base/0x7a2e753a20ad60a931643ce87aeadb8efbf1caa1/drops

If you can get the tests passing again today, we can get it merged today!

…inting into raribleIngestor

s29papi · 2024-08-27T12:51:35Z

Hi @chrismaddern. This is fixed, and the tests pass.

Sorry for the oversight.

Fixed it.

chrismaddern

Tests passing locally. Added RARIBLE_API_KEY to workflow and environment secrets

chrismaddern · 2024-08-27T13:56:54Z

@s29papi merging and sending you an email for bounty claim!! 🥳

chrismaddern · 2024-08-27T13:57:05Z

/review

qodo-merge-pro · 2024-08-27T13:57:27Z

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 Security concerns API key exposure: The Rarible API key is being used directly in the code in src/ingestors/rarible/offchain-metadata.ts. This could potentially lead to the exposure of sensitive information. It's recommended to use environment variables or a more secure method to handle API keys.
⚡ Key issues to review Error Handling The error handling in the `createMintTemplateForUrl` and `createMintForContract` methods could be improved. Consider adding more specific error types and handling them separately. API Key Security The Rarible API key is being used directly in the code. Consider using environment variables or a more secure method to handle API keys. Error Handling The error handling in the `raribleContractMetadataGetter` function could be improved. Consider adding more specific error types and handling them separately.

raribleIngestor

56affa0

s29papi mentioned this pull request Jul 5, 2024

Onchain Summer Bounty → Rarible Base Mint Ingestor #10

Closed

raribleIngestor closes floornfts#10

42e655c

chrismaddern reviewed Jul 16, 2024

View reviewed changes

Merge remote-tracking branch 'origin/HEAD' into raribleIngestor

9c18f65

s29papi added 2 commits July 23, 2024 02:50

resolved metadata issues, added simulation testing

9000f46

Merge remote-tracking branch 'origin/HEAD' into raribleIngestor

515d5e6

chrismaddern reviewed Jul 23, 2024

View reviewed changes

added method: raribleCreatorProfileDataGetter

1d8fee4

chrismaddern added the final-approach label Jul 24, 2024

s29papi added 4 commits July 25, 2024 17:29

Merge branch 'main' into raribleIngestor

322d567

Merge branch 'main' into raribleIngestor

30b35bf

switch to urls with common mint timestamp

19ad998

Merge branch 'raribleIngestor' of https://github.com/s29papi/mobile-m…

1ea046b

…inting into raribleIngestor

cleanup

0fb1897

Merge branch 'main' into raribleIngestor

920e507

s29papi added 2 commits August 27, 2024 13:33

changed stale claim stop timestamp

0aee8b3

Merge branch 'raribleIngestor' of https://github.com/s29papi/mobile-m…

2da2a0c

…inting into raribleIngestor

Add RARIBLE_API_KEY to tests script

ef5ea79

chrismaddern approved these changes Aug 27, 2024

View reviewed changes

chrismaddern added accepted and removed final-approach labels Aug 27, 2024

qodo-merge-pro bot added the Review effort [1-5]: 4 label Aug 27, 2024

chrismaddern merged commit 31d6265 into floornfts:main Aug 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

raribleIngestor #24

raribleIngestor #24

s29papi commented Jul 5, 2024

s29papi commented Jul 10, 2024

chrismaddern commented Jul 16, 2024

chrismaddern Jul 16, 2024

s29papi Jul 16, 2024

chrismaddern Jul 16, 2024

chrismaddern Jul 16, 2024

chrismaddern Jul 16, 2024

chrismaddern Jul 16, 2024

s29papi Jul 16, 2024

chrismaddern Jul 16, 2024

chrismaddern commented Jul 16, 2024

s29papi commented Jul 16, 2024

chrismaddern commented Jul 19, 2024

s29papi commented Jul 22, 2024

chrismaddern commented Jul 22, 2024

s29papi commented Jul 22, 2024

s29papi commented Jul 23, 2024

chrismaddern Jul 23, 2024

s29papi Jul 23, 2024

chrismaddern commented Jul 23, 2024

s29papi commented Jul 23, 2024

s29papi commented Jul 26, 2024

s29papi commented Aug 12, 2024

chrismaddern commented Aug 27, 2024

s29papi commented Aug 27, 2024

chrismaddern left a comment

chrismaddern commented Aug 27, 2024

chrismaddern commented Aug 27, 2024

qodo-merge-pro bot commented Aug 27, 2024

raribleIngestor #24

raribleIngestor #24

Conversation

s29papi commented Jul 5, 2024

s29papi commented Jul 10, 2024

chrismaddern commented Jul 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrismaddern commented Jul 16, 2024

s29papi commented Jul 16, 2024

chrismaddern commented Jul 19, 2024

s29papi commented Jul 22, 2024

chrismaddern commented Jul 22, 2024

s29papi commented Jul 22, 2024

s29papi commented Jul 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrismaddern commented Jul 23, 2024

s29papi commented Jul 23, 2024

s29papi commented Jul 26, 2024

s29papi commented Aug 12, 2024

chrismaddern commented Aug 27, 2024

s29papi commented Aug 27, 2024

chrismaddern left a comment

Choose a reason for hiding this comment

chrismaddern commented Aug 27, 2024

chrismaddern commented Aug 27, 2024

qodo-merge-pro bot commented Aug 27, 2024

PR Reviewer Guide 🔍