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

Fix crossChainTransfer for the changes in adapter. fix #137 #138

Merged
merged 11 commits into from
Dec 28, 2023

Conversation

imstar15
Copy link
Member

@imstar15 imstar15 commented Dec 22, 2023

  1. Fix address issue in cross-chain transfer
  2. Add Moonbeam crossChainTransferWithEthSigner function. It is implemented by calling the xtokens contract on Moonriver.
  3. Add Astar crossChainTransferWithEthSigner function. It is implemented by calling the xcm contract on Shiden.

Copy link

changeset-bot bot commented Dec 22, 2023

⚠️ No Changeset found

Latest commit: b82856a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
ethers 6.9.1 network, filesystem +10 22.2 MB ricmoo

@imstar15 imstar15 changed the title [WIP]Fix address issue in cross-chain transfer Fix crossChainTransfer for the changes in adapter. fix #137 Dec 26, 2023
@imstar15 imstar15 marked this pull request as ready for review December 26, 2023 09:32
Copy link
Member

@chrisli30 chrisli30 left a comment

Choose a reason for hiding this comment

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

I have added a few suggestions.

};

// eslint-disable-next-line import/no-default-export
export default xtokens;
Copy link
Member

Choose a reason for hiding this comment

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

If there are multiple exports, we can use export {abi, address}. That’s why eslint recommends import/no-default-export.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

const moonbeam = { xtokens };

// eslint-disable-next-line import/no-default-export
export default moonbeam;
Copy link
Member

Choose a reason for hiding this comment

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

export { moonbeam }; since we will add more chains to this object later, such as {acala, moonbeam, astar}

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

const contracts = { astar, moonbeam };

// eslint-disable-next-line import/no-default-export
export default contracts;
Copy link
Member

Choose a reason for hiding this comment

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

Try the below lines,

import * from `./astar`;
import * from `./moonbeam`;

export {astar, moonbeam};

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

/**
* Determine the account type based on the address.
* Because 1 byte takes 2 characters in hex string, we can use the length of the address to determine the address type.
* AccountKey20: 42 characters with prefix 0x.
Copy link
Member

Choose a reason for hiding this comment

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

Let’s add the 0x check to this function.

const length = _.startWith(address, "0x")? 42: 40;

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

* Determine the account type based on the address.
* Because 1 byte takes 2 characters in hex string, we can use the length of the address to determine the address type.
* AccountKey20: 42 characters with prefix 0x.
* AccountId32: 66 characters with prefix 0x.
Copy link
Member

Choose a reason for hiding this comment

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

No substrate wallet address contains "0x", so AccountId32 length check should be 64. This function needs to throw an error on all wrong cases instead of using AccountType.AccountId32 as the fall-back.

For example,

if starts with 0x
   if length is 42
       AccountType.AccountKey20
else
   if length is 40
       AccountType.AccountKey20
   if length is 64
       AccountType.AccountId32

throw new Error(`Unrecognized address format`);

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Member

@chrisli30 chrisli30 left a comment

Choose a reason for hiding this comment

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

Okay, the changes look good 👌

@imstar15 imstar15 merged commit 431e087 into dev Dec 28, 2023
2 checks passed
@imstar15 imstar15 deleted the fix-address-in-cross-chain-transfer branch December 28, 2023 07:27
imstar15 added a commit that referenced this pull request Jul 17, 2024
* Determine the account type based on the address.

* Cross-chain transfer with eth signer

* Convert location to precompile multi-location

* Add docs references for convertLocationToPrecompileMultiLocation function

* Add astar crossChainTransferWithEthSigner function

* Set the correct unlimitedWeight when calling astar contract call

* Move contract utils to contract-utils.ts

* Fix the error in finding the selector

* Modify the export method of contracts

* Fix the conversion from bytes to bits

* An example explaining how to calculate the accountId32 from a Substrate address
chrisli30 pushed a commit that referenced this pull request Jul 18, 2024
* Determine the account type based on the address.

* Cross-chain transfer with eth signer

* Convert location to precompile multi-location

* Add docs references for convertLocationToPrecompileMultiLocation function

* Add astar crossChainTransferWithEthSigner function

* Set the correct unlimitedWeight when calling astar contract call

* Move contract utils to contract-utils.ts

* Fix the error in finding the selector

* Modify the export method of contracts

* Fix the conversion from bytes to bits

* An example explaining how to calculate the accountId32 from a Substrate address
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.

2 participants