Skip to content
This repository has been archived by the owner on Sep 28, 2023. It is now read-only.

Feature xcmp assets #35

Merged
merged 7 commits into from Apr 26, 2022
Merged

Feature xcmp assets #35

merged 7 commits into from Apr 26, 2022

Conversation

akru
Copy link
Member

@akru akru commented Apr 8, 2022 •
edited

Pull Request Summary

This PR introduces XC20 precompiles adopted to be used in Astar ecosystem.

Check list

  • contains breaking changes
  • adds new feature
  • modifies existing feature (bug fix or improvements)
  • relies on other tasks
  • documentation changes
  • tests and/or benchmarks are included
  • changed API client type definition or chain metadata

This pull request makes the following changes:

Adds

  • (ex: Add feature A)

Fixes

  • (ex: Fix validation function)

Changes

  • (ex: Change document B)

To-dos

*Feel free to remove this section if it's not applicable

  • (ex: add user list)

@akru akru requested a review from Dinonard April 8, 2022 13:47
@akru akru requested a review from hoonsubin April 11, 2022 06:40
hoonsubin
hoonsubin approved these changes Apr 11, 2022
View reviewed changes
Copy link

@hoonsubin hoonsubin left a comment •
edited

Choose a reason for hiding this comment

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

So this PR is about importing MB ERC20 precompiled? I think we should change the PR title to reflect that.

LGTM!

(please resolve the unit test issue)

Dinonard
Dinonard suggested changes Apr 11, 2022
View reviewed changes
Copy link
Member

@Dinonard Dinonard left a comment

Choose a reason for hiding this comment

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

Please fix the UT.

precompiles/assets-erc20/src/lib.rs Outdated
/// The following distribution has been decided for the precompiles
/// 0-1023: Ethereum Mainnet Precompiles
/// 1024-2047 Precompiles that are not in Ethereum Mainnet but are neither Moonbeam specific
/// 2048-4095 Moonbeam specific precompiles
Copy link
Member

Choose a reason for hiding this comment

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

Please change this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed

Copy link
Member

Choose a reason for hiding this comment

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

Great!

@akru
Copy link
Member Author

akru commented Apr 25, 2022

@Dinonard please review, most comments fixed.

@akru akru mentioned this pull request Apr 26, 2022
Merged
9 tasks
Dinonard
Dinonard approved these changes Apr 26, 2022
View reviewed changes
Copy link
Member

@Dinonard Dinonard left a comment

Choose a reason for hiding this comment

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

LGTM

@akru akru merged commit 688d833 into polkadot-v0.9.17 Apr 26, 2022
@akru akru deleted the feature/xcmp-assets branch April 26, 2022 08:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Dinonard Dinonard Dinonard approved these changes

@hoonsubin hoonsubin hoonsubin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@akru @Dinonard @hoonsubin