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

CLI: Derive Account #860

Merged
merged 3 commits into from
Apr 5, 2021
Merged

CLI: Derive Account #860

merged 3 commits into from
Apr 5, 2021

Conversation

tomusdrw
Copy link
Contributor

@tomusdrw tomusdrw commented Apr 2, 2021

Related #855

This PR moves derive-account sub-command to its own module.

Comment on lines 72 to 74
let acc = bp_runtime::SourceAccount::Account(account.raw_id());
let id = bp_millau::derive_account_from_rialto_id(acc);
let derived_account = AccountId::from_raw::<Target>(id);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should't this be part of the macro code? For example, in the case of Millau to Rialto we would want:

let id = bp_rialto::derive_account_from_millau_id(acc);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great catch, I trusted myself too much that after #849 the code sections are properly split. It didn't trigger any compilation errors, cause the account types are the same on both chains.

@HCastano HCastano merged commit 23f1000 into master Apr 5, 2021
@HCastano HCastano deleted the td-derive-account branch April 5, 2021 15:58
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Mar 27, 2024
* Move derive account.

* Fix account derivation.
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Apr 8, 2024
* Move derive account.

* Fix account derivation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants