Skip to content
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

Rodeo ingestion #36

Merged
merged 11 commits into from
Aug 22, 2024
Merged

Rodeo ingestion #36

merged 11 commits into from
Aug 22, 2024

Conversation

Myestery
Copy link
Contributor

@Myestery Myestery commented Aug 9, 2024

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
Contributor

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
Contributor Author

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
@Myestery
Copy link
Contributor Author

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

@Myestery
Copy link
Contributor Author

Screenshot 2024-08-12 at 22 50 33

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

/review

Copy link

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.

Copy link
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
Contributor

DMd you on twitter 😊

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

Successfully merging this pull request may close these issues.

Bounty → Rodeo.Club Mint Ingestor
3 participants