Skip to content
This repository has been archived by the owner on May 9, 2024. It is now read-only.

Add ERC1155 support #289

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add ERC1155 support #289

wants to merge 4 commits into from

Conversation

axvn
Copy link
Contributor

@axvn axvn commented Mar 20, 2022

Description

Added ERC1155 support, this is built on the ERC20 data format 0x[tokenId][amount][addressLen][address], rather than using the batchMint data format proposed by the ERC1155Handler, corresponding changes needed in the ChainBridge-solidity repository.

Related Issue Or Context

Changed ERC1155Handler to use the simpler mint function.

ChainSafe/chainbridge-solidity#516

How Has This Been Tested? Testing details.

UnitTest provided, including go wrappers to the new handler.

Live demo can be tried out here: http://13.250.39.184:3001 (bridge between Mumbai and Rinkeby), you can get test tokens from opensea testnet.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation

Checklist:

  • I have commented my code, particularly in hard-to-understand areas.
  • I have ensured that all acceptance criteria (or expected behavior) from issue are met
  • I have updated the documentation locally and in chainbridge-docs.
  • I have added tests to cover my changes.
  • I have ensured that all the checks are passing and green, I've signed the CLA bot

@CLAassistant
Copy link

CLAassistant commented Mar 20, 2022

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@mpetrun5 mpetrun5 left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the contribution!

In general looks good to me I would just like to see an an integration test here:

func (s *IntegrationTestSuite) TestErc721Deposit() {

@mpetrun5
Copy link
Member

I would wait until the solidity team approves the solidity changes first before merging this.

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

Successfully merging this pull request may close these issues.

3 participants