Skip to content

Created Contract Wrappers#183

Merged
andygruening merged 6 commits intomasterfrom
feature/tokenfunctionswrappers
Oct 29, 2024
Merged

Created Contract Wrappers#183
andygruening merged 6 commits intomasterfrom
feature/tokenfunctionswrappers

Conversation

@caballoninja
Copy link
Copy Markdown
Contributor

Created Contract wrappers for erc20, erc721, erc1155, with grantrole, mint and burn. Also, erc721Sale, erc1155Sale with "PrimaryPurchase"/mint

Created Contract wrappers for erc20, erc721, erc1155, with grantrole, mint and burn. Also, erc721Sale, erc1155Sale with "PrimaryPurchase"/mint
Comment thread Plugins/SequencePlugin/Source/SequencePlugin/Private/Types/ERC1155.cpp Outdated
Copy link
Copy Markdown
Contributor

@BellringerQuinn BellringerQuinn left a comment

Choose a reason for hiding this comment

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

This is great! Thanks @caballoninja!

A couple of thoughts:

  • These are likely to be very high-touch wrappers; we should make sure to add them to our docs (including some sample code)
  • Similarly, as these are high-touch points; we should add some tests for them (@ZemindJan @caballoninja would you mind reviewing #172 when you get a chance)

@BellringerQuinn
Copy link
Copy Markdown
Contributor

Also, can we add constructors to the contract wrappers to require the contract address? It seems like every method call relies upon that info so I think it would be best for us to force it get set

revisions on constructors and fixes
Revisions
@andygruening
Copy link
Copy Markdown
Contributor

What was the update on the GetPaymentToken and GetSaleDetails functions for the sale contract wrappers? Right now theyre removed, or shall we change them to return RawTransactions instead?

@caballoninja
Copy link
Copy Markdown
Contributor Author

What was the update on the GetPaymentToken and GetSaleDetails functions for the sale contract wrappers? Right now theyre removed, or shall we change them to return RawTransactions instead?

(wouldnt let me answer directly) my idea is to revisit the contract call to make it blueprintable, but if not yes, i will make them rawTx

Included contractcalls
@caballoninja
Copy link
Copy Markdown
Contributor Author

@andygruening added back the contract calls

Copy link
Copy Markdown
Contributor

@BellringerQuinn BellringerQuinn left a comment

Choose a reason for hiding this comment

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

Overall looks good to me. I'll let @andygruening approve as he has more context with this ticket.

Question for you @caballoninja - I noticed that a lot of the contracts have an FString Data field. What is this for?

Comment thread Plugins/SequencePlugin/Source/SequencePlugin/Public/Types/ERC20.h
Comment thread Plugins/SequencePlugin/Source/SequencePlugin/Public/Types/ERC721.h
Comment thread Plugins/SequencePlugin/Source/SequencePlugin/Public/Types/ERC1155.h
andygruening
andygruening previously approved these changes Oct 28, 2024
@colezemind colezemind linked an issue Oct 28, 2024 that may be closed by this pull request
INcluded make approve transaction for erc20,721,1155
Copy link
Copy Markdown
Contributor

@andygruening andygruening left a comment

Choose a reason for hiding this comment

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

Great thanks, looks toptop!

@andygruening andygruening merged commit 8c94d4c into master Oct 29, 2024
@andygruening andygruening deleted the feature/tokenfunctionswrappers branch October 29, 2024 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add token wrappers for ERC20, ERC721, ERC1155s

3 participants