-
Notifications
You must be signed in to change notification settings - Fork 335
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
Adds H160 account #78
Conversation
Based on Basti's comment in paritytech/cumulus#226 (comment), we have two options for how to deal with the token dealer pallet. We can either hack it in our own repo to meet our needs, or we can omit it from our runtime entirely, knowing that we'll add something more permanent back when it is ready. |
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.
I made a first pass leaving some comments. The node compiles and runs without error for me, and I can deploy an ERC20.
FYI, the following type allows PolkadotJS to at least display our addresses
"AccountId": "[u8; 20]"
pub enum MultiSignature { | ||
Ed25519(ed25519::Signature), | ||
Sr25519(sr25519::Signature), | ||
Ecdsa(EthereumSignature), | ||
} |
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.
Do we have any desire to use ed25519 or sr25519? If not, this should definitely go.
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.
Do you think of any usecase to support those so far?
Otherwise yes, iit should go but we don't have a way to support it on PolkadotJs.
"balances": [ | ||
["15ozwjkTsjscc8raJFMdoEkzxE8NeqWWXwi4JGeC3DzAbGoi", 115292150460684697600], | ||
["16f4zjGB1XyXfdr7swiJFr91mSnHrb4CtEt95pz3htF8fHGQ", 115292150460684697600] | ||
] | ||
"balances": [] |
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.
Are you intentionally removing these prefunded accounts?
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.
I moved them into new account (under EVM).
const polkadotApi = await ApiPromise.create({ | ||
provider: wsProvider, | ||
types: { | ||
AccountId: "EthereumAccountId", | ||
Address: "AccountId", | ||
Balance: "u128", | ||
RefCount: "u8", | ||
// mapping the lookup | ||
LookupSource: "AccountId", | ||
Account: { | ||
nonce: "U256", | ||
balance: "u128", | ||
}, | ||
Transaction: { | ||
nonce: "U256", | ||
action: "String", | ||
gas_price: "u64", | ||
gas_limit: "u64", | ||
value: "U256", | ||
input: "Vec<u8>", | ||
signature: "Signature", | ||
}, | ||
Signature: { | ||
v: "u64", | ||
r: "H256", | ||
s: "H256", | ||
}, | ||
}, | ||
}); | ||
|
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.
These should be taken from the dedicated files rather than added here explicitly
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.
It will once the polkadotjs types pr is approved
Adds Account20 and Signer for ECDSA(Ethereum)
In order to support current PolkadotJs SDK (and possibly other tools), we need to ensure the node supports MultiSignature with the same number of Signature defined, even if they are not supported (they gets refused when verifying them). This is due to the use of a enum at the begining of the signature to specify which Signature is used.
Default accounts have been removed as they are not compatible with AccountId20 (Their accountId don't match anymore their privateKey). For now, we switched to use Gerald account (
0x6Be02d1d3665660d22FF9624b7BE0551ee1Ac91b
)