Skip to content

Rodeo ingestion#36

Merged
chrismaddern merged 11 commits intofloornfts:mainfrom
Myestery:rodeo-ingestor
Aug 22, 2024
Merged

Rodeo ingestion#36
chrismaddern merged 11 commits intofloornfts:mainfrom
Myestery:rodeo-ingestor

Conversation

@Myestery
Copy link
Copy Markdown
Contributor

@Myestery Myestery commented Aug 9, 2024 •
edited
Loading

Mint Ingestor: Rodeo Club

Functionality Supported

  • Ingesting from URL: Yes
  • Ingesting from Contract address: Yes
  • Supported Networks: Base

Before you submit

  • Ensure your generated MintTemplate works 😄
  • Ensure that your code is restricted to a single folder in src/ingestors
  • Ensure that all required assets are included (e.g. ABIs)
  • Ensure ABIs are trimmed to include only methods (1) used in the ingestor or (2) required to mint
  • Ensure that all exported methods are prefixed with the name of your ingestor e.g. myMintingPlatformGetContractDetails
  • Ensure that a test exists for generating a MintTemplate that will always succeed
  • Ensure that your code accesses no external resources except those passed in the resources object

fix #35

@chrismaddern
Copy link
Copy Markdown
Contributor

chrismaddern commented Aug 11, 2024

Hey @Myestery 👋

Excited to see you hopping in on another one!!

Just a headsup that we've gotten a few questions from other folks wanting to jump in here too, so for incomplete submissions we'll only hold the bounty for the first submitter for a few days.

Just wanted to give you a heads up since you were first, but it's not built yet.

@Myestery
Copy link
Copy Markdown
Contributor Author

Myestery commented Aug 11, 2024

oh yeah
I understand that and would turn this into a real PR tomorrow
I already worked on it during the weekend

@Myestery Myestery changed the title start rodeo ingestion Rodeo ingestion Aug 12, 2024
@Myestery Myestery marked this pull request as ready for review August 12, 2024 01:06
christinehall
christinehall reviewed Aug 12, 2024
View reviewed changes
@Myestery
Copy link
Copy Markdown
Contributor Author

Myestery commented Aug 12, 2024

Hello @christinehall This PR should ready for merge now
I've handled every issue that there was

@Myestery
Copy link
Copy Markdown
Contributor Author

Myestery commented Aug 12, 2024

Screenshot 2024-08-12 at 22 50 33

@Myestery Myestery requested a review from christinehall August 14, 2024 22:50
@chrismaddern
Copy link
Copy Markdown
Contributor

chrismaddern commented Aug 22, 2024

/review

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Aug 22, 2024

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Key issues to review

Hardcoded Address
The mintAddress is hardcoded in the createMintForContract method. This could be a potential issue if the address changes or if different addresses are used for different environments.

Error Handling
The error handling in the getRodeoMintByAddressAndChain function could be improved. It currently catches all errors and throws a generic MintIngestorError. More specific error handling could provide better debugging information.

Potential Overflow
The getRodeoFeeInEth function adds several BigNumber values. There's a potential for overflow if these values are extremely large. Consider using BigNumber.add with appropriate error checking.

chrismaddern
chrismaddern approved these changes Aug 22, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@chrismaddern chrismaddern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Congratulations on a single-review accepted PR!!!

Great job! Would love to see more of these 🥳

@chrismaddern chrismaddern merged commit b3e3bd4 into floornfts:main Aug 22, 2024
@chrismaddern
Copy link
Copy Markdown
Contributor

chrismaddern commented Aug 22, 2024

DMd you on twitter 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chrismaddern chrismaddern chrismaddern approved these changes

@christinehall christinehall Awaiting requested review from christinehall

Assignees

No one assigned

Labels

Review effort [1-5]: 3

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bounty → Rodeo.Club Mint Ingestor

3 participants

@Myestery @chrismaddern @christinehall