-
Notifications
You must be signed in to change notification settings - Fork 336
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
pallet-migrations: try-runtime support, and manage author mapping blake2 migration #796
Conversation
Co-authored-by: Joshy Orndorff <JoshOrndorff@users.noreply.github.com>
This is building and appears to be working now (with Joshy's I was able to test with diff --git a/utils/frame/remote-externalities/src/lib.rs b/utils/frame/remote-externalities/src/lib.rs
index 53c44780a6..8d1a686608 100644
--- a/utils/frame/remote-externalities/src/lib.rs
+++ b/utils/frame/remote-externalities/src/lib.rs
@@ -41,7 +41,7 @@ type KeyPair = (StorageKey, StorageData);
const LOG_TARGET: &str = "remote-ext";
const DEFAULT_TARGET: &str = "wss://rpc.polkadot.io";
-const BATCH_SIZE: usize = 1000;
+const BATCH_SIZE: usize = 100;
jsonrpsee_proc_macros::rpc_client_api! {
RpcApi<B: BlockT> { Without this, I found that the requests would consistently time out at the same batch. This was when using a locally-synced archive node (requests going over loopback). |
@@ -237,7 +239,7 @@ pub mod pallet { | |||
#[pallet::getter(fn account_and_deposit_of)] | |||
/// We maintain a mapping from the AuthorIds used in the consensus layer | |||
/// to the AccountIds runtime (including this staking pallet). | |||
type MappingWithDeposit<T: Config> = StorageMap< | |||
pub type MappingWithDeposit<T: Config> = StorageMap< |
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.
We are only using this in the crate context, we can make it pub crate only
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.
That's a valid point, but our other pallets also have them just pub
(eg https://github.com/PureStake/moonbeam/blob/master/pallets/asset-manager/src/lib.rs#L119). And I worry that making stuff too restricted will make it hard to write precompiles in the short term.
Here's a followup issue to treat this throughout the repo. https://purestake.atlassian.net/browse/MOON-933
What does it do?
This adds
try-runtime
support inpallet-migrations
so that the feature can be used as it would in a normal migration.What important points reviewers should know?
This PR currently uses
notlesh-migration-sketch
branch as the target for merging. I did this so we have the option of merging the other without this one.Is there something left for follow-up PRs?
The motivation for this is, in part, to allow #679 to use both
try-runtime
andpallet-migrations
.TODO:
post_upgrade()
only runs migrations that weren't skipped (see comments)cargo test
, currently requirescargo test --features=try-runtime
(see Activating features for tests/benchmarks rust-lang/cargo#2911)