-
Notifications
You must be signed in to change notification settings - Fork 154
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
JSSDK: EvmAccountMapping pallet support #1431
Merged
Leechael
merged 32 commits into
jssdk-v060-develop
from
feat-jssdk-evm-account-maaping-pallet
Dec 25, 2023
Merged
JSSDK: EvmAccountMapping pallet support #1431
Leechael
merged 32 commits into
jssdk-v060-develop
from
feat-jssdk-evm-account-maaping-pallet
Dec 25, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pacoyang
reviewed
Nov 13, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
38e4ea4
to
1ffc112
Compare
Conceptual ACK |
jasl
approved these changes
Nov 17, 2023
kvinwang
approved these changes
Nov 24, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
47d40f2
to
f3cc367
Compare
f3cc367
to
40955fd
Compare
Base automatically changed from
fix-sdk-cluster-workers-query-failed
to
master
November 26, 2023 14:35
40955fd
to
7fe6c8c
Compare
f88f66b
to
391a6ce
Compare
…s into substrate address
… unstable_provider
3b024a8
to
63d08f6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR propose to added experimental support to the evm_account_mapping. We added
getClient
andgetContract
to simplify the initialization process.getClient
andgetContract
In this PR, we added
getClient
andgetContract
as an experimental feature to simplify the overall APIs. The old approach is still effective, but we will not promote the new API until the overall API design is stable.getClient
is intended to avoid directly callingApiPromise
andOnChainRegistry
. Instead of:Now it can be just oneline:
The
client
is the instance ofOnChainRegistry
. If you need access theApiPromise
instance:getContract
is the factory function for thePinkContractPromise
object:evm_account_mapping
pallet supportCurrently, almost all polkadot-js scripts have a helper function called
signAndSend
, and it is rare seem to directly writeapiPromise.tx.foo.bar().signAndSend
. This is probably because most people need to check the result of submitting the transaction and determine if it is in block before further processing. Therefore, in the SDK, we have provided an implementation ofsignAndSend
that considers type hiting and handles common status callbacks. This utility function also supports compatibility with browser extension injected signer and polkadot-js signer.You can find the implementation here: https://github.com/Phala-Network/phala-blockchain/blob/master/frontend/packages/sdk/src/utils/signAndSend.ts
So instead of this:
It is more common to write in this way:
When designing the API for
evm_account_mapping
pallet interaction, there may be significant changes required to polkadot-js ApiPromise. These changes would involve encoding the payload, signing with EIP-712, and submitting extrinct without signing. However, it is important to note that these modifications could potentially introduce bugs and incompatibilities. We created theunstable_EvmAccountMappingProvider
to handle cases where the widely appliedsignAndSend
utility function is used.So here is the code example of how it works:
It is simpler for
PinkContractPromise
because it is created from scratch, and we simply added the new provider as an option to thesend
method.We considered the design unstable, so we prefixed it with
unstable_
here. The issue should arise when there are more and more complaints for the business model change, like Account Abstraction, so in the future, we will redesign the SDK and reduce more boilerplate codes.