Skip to content

feat(persistence-ethereum): added support for ERC1155 #3901

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(persistence-ethereum): added support for ERC1155 #3901

wants to merge 1 commit into from

Conversation

@Harshdev098
Copy link

@Harshdev098 Harshdev098 commented May 10, 2025 •
edited
Loading

Closes: #3925

Implemented ERC1155 token in plugin-persistence-ethereum and updated the db integeration test

petermetz
petermetz approved these changes May 18, 2025
View reviewed changes
Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

@Harshdev098 LGTM, thank you. Please make sure to have @outSH review it as well prior to merging.

@outSH outSH mentioned this pull request May 26, 2025
Open
outSH
outSH requested changes May 26, 2025
View reviewed changes
Copy link
Contributor

@outSH outSH left a comment

Choose a reason for hiding this comment

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

Thanks, this looks really great!

But please update persistence-ethereum-functional.test.ts test to validate that the new functionality is working correctly. In general, you can search for ERC721 and adjust / add test to check the same for ERC1155.

Also, please change the commit and PR title to something matching our convention (e.g. feat(persistence-ethereum): add support for ERC1155), and add at least descriptions of the changes.

Also in description change fix: #3553 to Closes: #3925 (I've created a new subissue for this specific task, and this formula will close the issue once the PR is merged).

Once again, great job, thank you!

@Harshdev098
Copy link
Author

Okay will update it!

@Harshdev098 Harshdev098 changed the title Implemented ERC1155 token in plugin-persistence-ethereum feat(persistence-ethereum): added support for ERC1155 May 31, 2025
@Harshdev098 Harshdev098 force-pushed the erc-1155 branch 2 times, most recently from d59e747 to 2a9e910 Compare May 31, 2025 22:37
@Harshdev098
Copy link
Author

Hey @outSH, Sorry for the delay, was busy in my semester exams, I have updated the integeration test to include erc1155.
Can you please review it!

@RafaelAPB
Copy link
Contributor

@outSH @Harshdev098 will this PR be updated?

@Harshdev098
Copy link
Author

Harshdev098 commented Oct 18, 2025 •
edited
Loading

@RafaelAPB @outSH I have updated the PR can you please review it!

@RafaelAPB
Copy link
Contributor

@Harshdev098 thanks for your contribution!
While generically it seems good, I will defer to @outSH the decision because he has more context on this package.
Generally LGTM, but I have two main concerns:
-There are no new tests to assert this functionality is working well
-There is no documentation for the new functionality

Please fix those items prior to merge.
Thanks again!

outSH
outSH approved these changes Oct 23, 2025
View reviewed changes
Copy link
Contributor

@outSH outSH left a comment

Choose a reason for hiding this comment

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

LGTM but you must fix required checks before merge

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

Reviewers

@petermetz petermetz petermetz approved these changes

@outSH outSH outSH approved these changes

@takeutak takeutak Awaiting requested review from takeutak takeutak is a code owner

@izuru0 izuru0 Awaiting requested review from izuru0 izuru0 is a code owner

@TheJagpreet TheJagpreet Awaiting requested review from TheJagpreet

@VRamakrishna VRamakrishna Awaiting requested review from VRamakrishna VRamakrishna is a code owner

@sandeepnRES sandeepnRES Awaiting requested review from sandeepnRES sandeepnRES is a code owner

@RafaelAPB RafaelAPB Awaiting requested review from RafaelAPB RafaelAPB is a code owner

@AndreAugusto11 AndreAugusto11 Awaiting requested review from AndreAugusto11 AndreAugusto11 is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat(persistence-ethereum): add support for ERC1155

4 participants

@Harshdev098 @RafaelAPB @petermetz @outSH