diff --git a/.github/workflows/review-trigger.yml b/.github/workflows/review-trigger.yml
index 8b23dd30bb29ad7879543c064c3eb711cc87895d..061cf4ab09ed919f6de0a83f74a22c2062c3a258 100644
--- a/.github/workflows/review-trigger.yml
+++ b/.github/workflows/review-trigger.yml
@@ -21,6 +21,38 @@ jobs:
- name: Skip merge queue
if: ${{ contains(github.ref, 'gh-readonly-queue') }}
run: exit 0
+ - name: Get comments
+ id: comments
+ run: echo "bodies=$(gh pr view ${{ github.event.number }} --repo ${{ github.repository }} --json comments --jq '[.comments[].body]')" >> "$GITHUB_OUTPUT"
+ env:
+ GH_TOKEN: ${{ github.token }}
+ - name: Fail when author pushes new code
+ # Require new reviews when the author is pushing and he is not a member
+ if: |
+ github.event_name == 'pull_request_target' &&
+ github.event.action == 'synchronize' &&
+ github.event.sender.login == github.event.pull_request.user.login &&
+ github.event.pull_request.author_association != 'MEMBER'
+ run: |
+ # We get the list of reviewers who approved the PR
+ REVIEWERS=$(gh api repos/${{ github.repository }}/pulls/${{ github.event.number }}/reviews \
+ --jq '{reviewers: [.[] | select(.state == "APPROVED") | .user.login]}')
+
+ # We request them to review again
+ echo $REVIEWERS | gh api --method POST repos/${{ github.repository }}/pulls/${{ github.event.number }}/requested_reviewers --input -
+
+ echo "::error::Project needs to be reviewed again"
+ exit 1
+ env:
+ GH_TOKEN: ${{ github.token }}
+ - name: Comment requirements
+ # If the previous step failed and github-actions hasn't commented yet we comment instructions
+ if: failure() && !contains(fromJson(steps.comments.outputs.bodies), 'Review required! Latest push from author must always be reviewed')
+ run: |
+ gh pr comment ${{ github.event.number }} --repo ${{ github.repository }} --body "Review required! Latest push from author must always be reviewed"
+ env:
+ GH_TOKEN: ${{ github.token }}
+ COMMENTS: ${{ steps.comments.outputs.users }}
- name: Get PR number
env:
PR_NUMBER: ${{ github.event.pull_request.number }}
diff --git a/Cargo.lock b/Cargo.lock
index 5e7959cf918b76ec00c0bf43e5bb6b91813d9155..db87de219ef97fafac7cdc5155800ca344a25d57 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -9595,7 +9595,7 @@ dependencies = [
[[package]]
name = "pallet-assets"
-version = "29.0.0"
+version = "29.1.0"
dependencies = [
"frame-benchmarking",
"frame-support",
@@ -9961,6 +9961,7 @@ dependencies = [
"frame-support",
"frame-system",
"parity-scale-codec",
+ "pretty_assertions",
"scale-info",
"sp-api",
"sp-arithmetic",
diff --git a/bridges/bin/runtime-common/src/extensions/refund_relayer_extension.rs b/bridges/bin/runtime-common/src/extensions/refund_relayer_extension.rs
index f97b23ecaaa99fdaed0a54d00b2b89c14847750a..64ae1d0b669f2ea8fdfba0df73752a9b0f6e8aec 100644
--- a/bridges/bin/runtime-common/src/extensions/refund_relayer_extension.rs
+++ b/bridges/bin/runtime-common/src/extensions/refund_relayer_extension.rs
@@ -23,7 +23,7 @@ use crate::messages_call_ext::{
CallHelper as MessagesCallHelper, CallInfo as MessagesCallInfo, MessagesCallSubType,
};
use bp_messages::{LaneId, MessageNonce};
-use bp_relayers::{RewardsAccountOwner, RewardsAccountParams};
+use bp_relayers::{ExplicitOrAccountParams, RewardsAccountOwner, RewardsAccountParams};
use bp_runtime::{Chain, Parachain, ParachainIdOf, RangeInclusiveExt, StaticStrProvider};
use codec::{Codec, Decode, Encode};
use frame_support::{
@@ -589,7 +589,10 @@ where
);
},
RelayerAccountAction::Slash(relayer, slash_account) =>
- RelayersPallet::<T::Runtime>::slash_and_deregister(&relayer, slash_account),
+ RelayersPallet::<T::Runtime>::slash_and_deregister(
+ &relayer,
+ ExplicitOrAccountParams::Params(slash_account),
+ ),
}
Ok(())
diff --git a/bridges/modules/relayers/src/benchmarking.rs b/bridges/modules/relayers/src/benchmarking.rs
index 00c3814a4c38d9bf0f18b70c0eedc75c239b8ad0..ca312d44edfddd286eae1715655d538b6b00f070 100644
--- a/bridges/modules/relayers/src/benchmarking.rs
+++ b/bridges/modules/relayers/src/benchmarking.rs
@@ -106,7 +106,7 @@ benchmarks! {
let slash_destination = RewardsAccountParams::new(lane, *b"test", RewardsAccountOwner::ThisChain);
T::prepare_rewards_account(slash_destination, Zero::zero());
}: {
- crate::Pallet::<T>::slash_and_deregister(&relayer, slash_destination)
+ crate::Pallet::<T>::slash_and_deregister(&relayer, slash_destination.into())
}
verify {
assert!(!crate::Pallet::<T>::is_registration_active(&relayer));
diff --git a/bridges/modules/relayers/src/lib.rs b/bridges/modules/relayers/src/lib.rs
index ce66c9df48e01c829a55d03ffe4437c41f006a6b..7a3a0f9ea94cbe5768bf6ee8c850355193ea44f0 100644
--- a/bridges/modules/relayers/src/lib.rs
+++ b/bridges/modules/relayers/src/lib.rs
@@ -21,7 +21,8 @@
#![warn(missing_docs)]
use bp_relayers::{
- PaymentProcedure, Registration, RelayerRewardsKeyProvider, RewardsAccountParams, StakeAndSlash,
+ ExplicitOrAccountParams, PaymentProcedure, Registration, RelayerRewardsKeyProvider,
+ RewardsAccountParams, StakeAndSlash,
};
use bp_runtime::StorageDoubleMapKeyProvider;
use frame_support::fail;
@@ -242,7 +243,7 @@ pub mod pallet {
/// It may fail inside, but error is swallowed and we only log it.
pub fn slash_and_deregister(
relayer: &T::AccountId,
- slash_destination: RewardsAccountParams,
+ slash_destination: ExplicitOrAccountParams<T::AccountId>,
) {
let registration = match RegisteredRelayers::<T>::take(relayer) {
Some(registration) => registration,
@@ -259,7 +260,7 @@ pub mod pallet {
match T::StakeAndSlash::repatriate_reserved(
relayer,
- slash_destination,
+ slash_destination.clone(),
registration.stake,
) {
Ok(failed_to_slash) if failed_to_slash.is_zero() => {
diff --git a/bridges/modules/relayers/src/stake_adapter.rs b/bridges/modules/relayers/src/stake_adapter.rs
index 88af9b1877bfe85614f081ee66dbb28586b1d34b..7ba90d91dfd94e49bf0ff6ee8fcc06f80e287c41 100644
--- a/bridges/modules/relayers/src/stake_adapter.rs
+++ b/bridges/modules/relayers/src/stake_adapter.rs
@@ -17,7 +17,7 @@
//! Code that allows `NamedReservableCurrency` to be used as a `StakeAndSlash`
//! mechanism of the relayers pallet.
-use bp_relayers::{PayRewardFromAccount, RewardsAccountParams, StakeAndSlash};
+use bp_relayers::{ExplicitOrAccountParams, PayRewardFromAccount, StakeAndSlash};
use codec::Codec;
use frame_support::traits::{tokens::BalanceStatus, NamedReservableCurrency};
use sp_runtime::{traits::Get, DispatchError, DispatchResult};
@@ -55,11 +55,14 @@ where
fn repatriate_reserved(
relayer: &AccountId,
- beneficiary: RewardsAccountParams,
+ beneficiary: ExplicitOrAccountParams<AccountId>,
amount: Currency::Balance,
) -> Result<Currency::Balance, DispatchError> {
- let beneficiary_account =
- PayRewardFromAccount::<(), AccountId>::rewards_account(beneficiary);
+ let beneficiary_account = match beneficiary {
+ ExplicitOrAccountParams::Explicit(account) => account,
+ ExplicitOrAccountParams::Params(params) =>
+ PayRewardFromAccount::<(), AccountId>::rewards_account(params),
+ };
Currency::repatriate_reserved_named(
&ReserveId::get(),
relayer,
@@ -134,7 +137,11 @@ mod tests {
Balances::mint_into(&beneficiary_account, expected_balance).unwrap();
assert_eq!(
- TestStakeAndSlash::repatriate_reserved(&1, beneficiary, test_stake()),
+ TestStakeAndSlash::repatriate_reserved(
+ &1,
+ ExplicitOrAccountParams::Params(beneficiary),
+ test_stake()
+ ),
Ok(test_stake())
);
assert_eq!(Balances::free_balance(1), 0);
@@ -146,7 +153,11 @@ mod tests {
Balances::mint_into(&2, test_stake() * 2).unwrap();
TestStakeAndSlash::reserve(&2, test_stake() / 3).unwrap();
assert_eq!(
- TestStakeAndSlash::repatriate_reserved(&2, beneficiary, test_stake()),
+ TestStakeAndSlash::repatriate_reserved(
+ &2,
+ ExplicitOrAccountParams::Params(beneficiary),
+ test_stake()
+ ),
Ok(test_stake() - test_stake() / 3)
);
assert_eq!(Balances::free_balance(2), test_stake() * 2 - test_stake() / 3);
@@ -158,7 +169,11 @@ mod tests {
Balances::mint_into(&3, test_stake() * 2).unwrap();
TestStakeAndSlash::reserve(&3, test_stake()).unwrap();
assert_eq!(
- TestStakeAndSlash::repatriate_reserved(&3, beneficiary, test_stake()),
+ TestStakeAndSlash::repatriate_reserved(
+ &3,
+ ExplicitOrAccountParams::Params(beneficiary),
+ test_stake()
+ ),
Ok(0)
);
assert_eq!(Balances::free_balance(3), test_stake());
@@ -176,7 +191,12 @@ mod tests {
Balances::mint_into(&3, test_stake() * 2).unwrap();
TestStakeAndSlash::reserve(&3, test_stake()).unwrap();
- assert!(TestStakeAndSlash::repatriate_reserved(&3, beneficiary, test_stake()).is_err());
+ assert!(TestStakeAndSlash::repatriate_reserved(
+ &3,
+ ExplicitOrAccountParams::Params(beneficiary),
+ test_stake()
+ )
+ .is_err());
assert_eq!(Balances::free_balance(3), test_stake());
assert_eq!(Balances::reserved_balance(3), test_stake());
assert_eq!(Balances::free_balance(beneficiary_account), 0);
diff --git a/bridges/primitives/relayers/src/lib.rs b/bridges/primitives/relayers/src/lib.rs
index c808c437b54cbaaa5813067a6413fe7189336ee6..2a9ef6a8e1e9aba999ea90045447f7a87fb3813b 100644
--- a/bridges/primitives/relayers/src/lib.rs
+++ b/bridges/primitives/relayers/src/lib.rs
@@ -19,7 +19,7 @@
#![warn(missing_docs)]
#![cfg_attr(not(feature = "std"), no_std)]
-pub use registration::{Registration, StakeAndSlash};
+pub use registration::{ExplicitOrAccountParams, Registration, StakeAndSlash};
use bp_messages::LaneId;
use bp_runtime::{ChainId, StorageDoubleMapKeyProvider};
diff --git a/bridges/primitives/relayers/src/registration.rs b/bridges/primitives/relayers/src/registration.rs
index 38fa7c2d9075b7e84986eb4ff173cdb24db5610f..9d9b7e4812201390c6831ff020c12c8cc995c17a 100644
--- a/bridges/primitives/relayers/src/registration.rs
+++ b/bridges/primitives/relayers/src/registration.rs
@@ -46,6 +46,21 @@ use sp_runtime::{
DispatchError, DispatchResult,
};
+/// Either explicit account reference or `RewardsAccountParams`.
+#[derive(Clone, Debug)]
+pub enum ExplicitOrAccountParams<AccountId> {
+ /// Explicit account reference.
+ Explicit(AccountId),
+ /// Account, referenced using `RewardsAccountParams`.
+ Params(RewardsAccountParams),
+}
+
+impl<AccountId> From<RewardsAccountParams> for ExplicitOrAccountParams<AccountId> {
+ fn from(params: RewardsAccountParams) -> Self {
+ ExplicitOrAccountParams::Params(params)
+ }
+}
+
/// Relayer registration.
#[derive(Copy, Clone, Debug, Decode, Encode, Eq, PartialEq, TypeInfo, MaxEncodedLen)]
pub struct Registration<BlockNumber, Balance> {
@@ -90,7 +105,7 @@ pub trait StakeAndSlash<AccountId, BlockNumber, Balance> {
/// Returns `Ok(_)` with non-zero balance if we have failed to repatriate some portion of stake.
fn repatriate_reserved(
relayer: &AccountId,
- beneficiary: RewardsAccountParams,
+ beneficiary: ExplicitOrAccountParams<AccountId>,
amount: Balance,
) -> Result<Balance, DispatchError>;
}
@@ -113,7 +128,7 @@ where
fn repatriate_reserved(
_relayer: &AccountId,
- _beneficiary: RewardsAccountParams,
+ _beneficiary: ExplicitOrAccountParams<AccountId>,
_amount: Balance,
) -> Result<Balance, DispatchError> {
Ok(Zero::zero())
diff --git a/polkadot/tests/benchmark_block.rs b/polkadot/tests/benchmark_block.rs
index 99f95ef611a48266105ba6b413641867fab21101..bc268025985035a74996ba444e02b98471bb1125 100644
--- a/polkadot/tests/benchmark_block.rs
+++ b/polkadot/tests/benchmark_block.rs
@@ -58,7 +58,13 @@ async fn build_chain(runtime: &str, base_path: &Path) {
let mut cmd = Command::new(cargo_bin("polkadot"))
.stdout(process::Stdio::piped())
.stderr(process::Stdio::piped())
- .args(["--chain", runtime, "--force-authoring", "--alice"])
+ .args([
+ "--chain",
+ runtime,
+ "--force-authoring",
+ "--alice",
+ "--unsafe-force-node-key-generation",
+ ])
.arg("-d")
.arg(base_path)
.arg("--no-hardware-benchmarks")
diff --git a/prdoc/pr_3852.prdoc b/prdoc/pr_3852.prdoc
new file mode 100644
index 0000000000000000000000000000000000000000..f13e1766d518a14a0a5309e75937007bf937a79d
--- /dev/null
+++ b/prdoc/pr_3852.prdoc
@@ -0,0 +1,25 @@
+title: (Breaking change)Enforce network key presence on authorities.
+
+doc:
+ - audience: Node Operator
+ description: |
+ (Breaking change) For all authority nodes, the node binary now enforces the presence
+ of a network key, instead of auto-generating when it is absent.
+
+ Before this change, all node binaries were auto-generating the node key when it was not present,
+ that is dangerous because other nodes in the network expects a stable identity for authorities.
+
+ To prevent accidental generation of node key, we removed this behaviour and node binary will now throw
+ an error if the network key is not present and operators will receive instructions to either persist
+ their network key or explicitly generate a new one with the `polkadot key generate-node-key`.
+
+ To prevent this error on restart/upgrades node operators need to make sure their network key are always
+ persisted, if nodes already correctly persist all directories in `--base-path` then no action is needed.
+
+crates:
+ - name: sc-cli
+ bump: major
+ - name: polkadot
+ bump: major
+ - name: subkey
+ bump: minor
\ No newline at end of file
diff --git a/prdoc/pr_4089.prdoc b/prdoc/pr_4089.prdoc
new file mode 100644
index 0000000000000000000000000000000000000000..29ac736ccd082ec493b89824fab0f9519bd66c6d
--- /dev/null
+++ b/prdoc/pr_4089.prdoc
@@ -0,0 +1,11 @@
+title: "pallet_broker: Support renewing leases expired in a previous period"
+
+doc:
+ - audience: Runtime User
+ description: |
+ Allow renewals of leases that ended before `start_sales` or in the first period after calling `start_sales`.
+ This ensures that everyone has a smooth experience when migrating to coretime and the timing of
+ `start_sales` isn't that important.
+
+crates:
+ - name: pallet-broker
diff --git a/prdoc/pr_4118.prdoc b/prdoc/pr_4118.prdoc
new file mode 100644
index 0000000000000000000000000000000000000000..20f36c1b0a37cc330c23115daa59f7a6c14b3c6a
--- /dev/null
+++ b/prdoc/pr_4118.prdoc
@@ -0,0 +1,13 @@
+title: "pallet assets: minor improvement on errors returned for some calls"
+
+doc:
+ - audience: Runtime Dev
+ description: |
+ Some calls in pallet assets have better errors. No new error is introduced, only more sensible choice are made.
+ - audience: Runtime User
+ description: |
+ Some calls in pallet assets have better errors. No new error is introduced, only more sensible choice are made.
+
+crates:
+ - name: pallet-assets
+ bump: minor
diff --git a/substrate/bin/utils/subkey/src/lib.rs b/substrate/bin/utils/subkey/src/lib.rs
index 33f28ef46a5e7805b2c00b047e413f049169f6ef..0ca65cd08a6bcade0522b112a9bd327ed898badd 100644
--- a/substrate/bin/utils/subkey/src/lib.rs
+++ b/substrate/bin/utils/subkey/src/lib.rs
@@ -310,7 +310,7 @@
use clap::Parser;
use sc_cli::{
- Error, GenerateCmd, GenerateNodeKeyCmd, InspectKeyCmd, InspectNodeKeyCmd, SignCmd, VanityCmd,
+ Error, GenerateCmd, GenerateKeyCmdCommon, InspectKeyCmd, InspectNodeKeyCmd, SignCmd, VanityCmd,
VerifyCmd,
};
@@ -324,7 +324,7 @@ use sc_cli::{
pub enum Subkey {
/// Generate a random node key, write it to a file or stdout and write the
/// corresponding peer-id to stderr
- GenerateNodeKey(GenerateNodeKeyCmd),
+ GenerateNodeKey(GenerateKeyCmdCommon),
/// Generate a random account
Generate(GenerateCmd),
diff --git a/substrate/client/cli/src/commands/generate_node_key.rs b/substrate/client/cli/src/commands/generate_node_key.rs
index 43851dc1af5ccf555668e2014ba6fe57c31e858c..bdb94eec93b4a38797b54a7e7459d91dc88a3794 100644
--- a/substrate/client/cli/src/commands/generate_node_key.rs
+++ b/substrate/client/cli/src/commands/generate_node_key.rs
@@ -17,23 +17,19 @@
//! Implementation of the `generate-node-key` subcommand
-use crate::Error;
-use clap::Parser;
+use crate::{build_network_key_dir_or_default, Error, NODE_KEY_ED25519_FILE};
+use clap::{Args, Parser};
use libp2p_identity::{ed25519, Keypair};
+use sc_service::BasePath;
use std::{
fs,
io::{self, Write},
path::PathBuf,
};
-/// The `generate-node-key` command
-#[derive(Debug, Parser)]
-#[command(
- name = "generate-node-key",
- about = "Generate a random node key, write it to a file or stdout \
- and write the corresponding peer-id to stderr"
-)]
-pub struct GenerateNodeKeyCmd {
+/// Common arguments accross all generate key commands, subkey and node.
+#[derive(Debug, Args, Clone)]
+pub struct GenerateKeyCmdCommon {
/// Name of file to save secret key to.
/// If not given, the secret key is printed to stdout.
#[arg(long)]
@@ -45,32 +41,111 @@ pub struct GenerateNodeKeyCmd {
bin: bool,
}
-impl GenerateNodeKeyCmd {
+/// The `generate-node-key` command
+#[derive(Debug, Clone, Parser)]
+#[command(
+ name = "generate-node-key",
+ about = "Generate a random node key, write it to a file or stdout \
+ and write the corresponding peer-id to stderr"
+)]
+pub struct GenerateNodeKeyCmd {
+ #[clap(flatten)]
+ pub common: GenerateKeyCmdCommon,
+ /// Specify the chain specification.
+ ///
+ /// It can be any of the predefined chains like dev, local, staging, polkadot, kusama.
+ #[arg(long, value_name = "CHAIN_SPEC")]
+ pub chain: Option<String>,
+ /// A directory where the key should be saved. If a key already
+ /// exists in the directory, it won't be overwritten.
+ #[arg(long, conflicts_with_all = ["file", "default_base_path"])]
+ base_path: Option<PathBuf>,
+
+ /// Save the key in the default directory. If a key already
+ /// exists in the directory, it won't be overwritten.
+ #[arg(long, conflicts_with_all = ["base_path", "file"])]
+ default_base_path: bool,
+}
+
+impl GenerateKeyCmdCommon {
/// Run the command
pub fn run(&self) -> Result<(), Error> {
- let keypair = ed25519::Keypair::generate();
+ generate_key(&self.file, self.bin, None, &None, false, None)
+ }
+}
+
+impl GenerateNodeKeyCmd {
+ /// Run the command
+ pub fn run(&self, chain_spec_id: &str, executable_name: &String) -> Result<(), Error> {
+ generate_key(
+ &self.common.file,
+ self.common.bin,
+ Some(chain_spec_id),
+ &self.base_path,
+ self.default_base_path,
+ Some(executable_name),
+ )
+ }
+}
+
+// Utility function for generating a key based on the provided CLI arguments
+//
+// `file` - Name of file to save secret key to
+// `bin`
+fn generate_key(
+ file: &Option<PathBuf>,
+ bin: bool,
+ chain_spec_id: Option<&str>,
+ base_path: &Option<PathBuf>,
+ default_base_path: bool,
+ executable_name: Option<&String>,
+) -> Result<(), Error> {
+ let keypair = ed25519::Keypair::generate();
- let secret = keypair.secret();
+ let secret = keypair.secret();
- let file_data = if self.bin {
- secret.as_ref().to_owned()
- } else {
- array_bytes::bytes2hex("", secret).into_bytes()
- };
+ let file_data = if bin {
+ secret.as_ref().to_owned()
+ } else {
+ array_bytes::bytes2hex("", secret).into_bytes()
+ };
- match &self.file {
- Some(file) => fs::write(file, file_data)?,
- None => io::stdout().lock().write_all(&file_data)?,
- }
+ match (file, base_path, default_base_path) {
+ (Some(file), None, false) => fs::write(file, file_data)?,
+ (None, Some(_), false) | (None, None, true) => {
+ let network_path = build_network_key_dir_or_default(
+ base_path.clone().map(BasePath::new),
+ chain_spec_id.unwrap_or_default(),
+ executable_name.ok_or(Error::Input("Executable name not provided".into()))?,
+ );
- eprintln!("{}", Keypair::from(keypair).public().to_peer_id());
+ fs::create_dir_all(network_path.as_path())?;
- Ok(())
+ let key_path = network_path.join(NODE_KEY_ED25519_FILE);
+ if key_path.exists() {
+ eprintln!("Skip generation, a key already exists in {:?}", key_path);
+ return Err(Error::KeyAlreadyExistsInPath(key_path));
+ } else {
+ eprintln!("Generating key in {:?}", key_path);
+ fs::write(key_path, file_data)?
+ }
+ },
+ (None, None, false) => io::stdout().lock().write_all(&file_data)?,
+ (_, _, _) => {
+ // This should not happen, arguments are marked as mutually exclusive.
+ return Err(Error::Input("Mutually exclusive arguments provided".into()));
+ },
}
+
+ eprintln!("{}", Keypair::from(keypair).public().to_peer_id());
+
+ Ok(())
}
#[cfg(test)]
-mod tests {
+pub mod tests {
+ use crate::DEFAULT_NETWORK_CONFIG_PATH;
+
use super::*;
use std::io::Read;
use tempfile::Builder;
@@ -80,9 +155,32 @@ mod tests {
let mut file = Builder::new().prefix("keyfile").tempfile().unwrap();
let file_path = file.path().display().to_string();
let generate = GenerateNodeKeyCmd::parse_from(&["generate-node-key", "--file", &file_path]);
- assert!(generate.run().is_ok());
+ assert!(generate.run("test", &String::from("test")).is_ok());
let mut buf = String::new();
assert!(file.read_to_string(&mut buf).is_ok());
assert!(array_bytes::hex2bytes(&buf).is_ok());
}
+
+ #[test]
+ fn generate_node_key_base_path() {
+ let base_dir = Builder::new().prefix("keyfile").tempdir().unwrap();
+ let key_path = base_dir
+ .path()
+ .join("chains/test_id/")
+ .join(DEFAULT_NETWORK_CONFIG_PATH)
+ .join(NODE_KEY_ED25519_FILE);
+ let base_path = base_dir.path().display().to_string();
+ let generate =
+ GenerateNodeKeyCmd::parse_from(&["generate-node-key", "--base-path", &base_path]);
+ assert!(generate.run("test_id", &String::from("test")).is_ok());
+ let buf = fs::read_to_string(key_path.as_path()).unwrap();
+ assert!(array_bytes::hex2bytes(&buf).is_ok());
+
+ assert!(generate.run("test_id", &String::from("test")).is_err());
+ let new_buf = fs::read_to_string(key_path).unwrap();
+ assert_eq!(
+ array_bytes::hex2bytes(&new_buf).unwrap(),
+ array_bytes::hex2bytes(&buf).unwrap()
+ );
+ }
}
diff --git a/substrate/client/cli/src/commands/inspect_node_key.rs b/substrate/client/cli/src/commands/inspect_node_key.rs
index 6cf025a2d115066ed18277e4036a028965e3c32f..25a0a685650e3d39709e7f8e9af903d7d72070d0 100644
--- a/substrate/client/cli/src/commands/inspect_node_key.rs
+++ b/substrate/client/cli/src/commands/inspect_node_key.rs
@@ -79,7 +79,9 @@ impl InspectNodeKeyCmd {
#[cfg(test)]
mod tests {
- use super::{super::GenerateNodeKeyCmd, *};
+ use crate::commands::generate_node_key::GenerateNodeKeyCmd;
+
+ use super::*;
#[test]
fn inspect_node_key() {
@@ -87,7 +89,7 @@ mod tests {
let path = path.to_str().unwrap();
let cmd = GenerateNodeKeyCmd::parse_from(&["generate-node-key", "--file", path]);
- assert!(cmd.run().is_ok());
+ assert!(cmd.run("test", &String::from("test")).is_ok());
let cmd = InspectNodeKeyCmd::parse_from(&["inspect-node-key", "--file", path]);
assert!(cmd.run().is_ok());
diff --git a/substrate/client/cli/src/commands/key.rs b/substrate/client/cli/src/commands/key.rs
index d49b7e4072c8eb5453b0540117dacaf23e89f912..52747b404622691cc0ef572ab91f39e3d23a3eb5 100644
--- a/substrate/client/cli/src/commands/key.rs
+++ b/substrate/client/cli/src/commands/key.rs
@@ -47,7 +47,10 @@ impl KeySubcommand {
/// run the key subcommands
pub fn run<C: SubstrateCli>(&self, cli: &C) -> Result<(), Error> {
match self {
- KeySubcommand::GenerateNodeKey(cmd) => cmd.run(),
+ KeySubcommand::GenerateNodeKey(cmd) => {
+ let chain_spec = cli.load_spec(cmd.chain.as_deref().unwrap_or(""))?;
+ cmd.run(chain_spec.id(), &C::executable_name())
+ },
KeySubcommand::Generate(cmd) => cmd.run(),
KeySubcommand::Inspect(cmd) => cmd.run(),
KeySubcommand::Insert(cmd) => cmd.run(cli),
diff --git a/substrate/client/cli/src/commands/mod.rs b/substrate/client/cli/src/commands/mod.rs
index 9d48d2bdf644fdc2ec6417148f138aafaf04f063..2d7a0dc72ff5355d2541ea098c140878649accdb 100644
--- a/substrate/client/cli/src/commands/mod.rs
+++ b/substrate/client/cli/src/commands/mod.rs
@@ -42,7 +42,7 @@ mod verify;
pub use self::{
build_spec_cmd::BuildSpecCmd, chain_info_cmd::ChainInfoCmd, check_block_cmd::CheckBlockCmd,
export_blocks_cmd::ExportBlocksCmd, export_state_cmd::ExportStateCmd, generate::GenerateCmd,
- generate_node_key::GenerateNodeKeyCmd, import_blocks_cmd::ImportBlocksCmd,
+ generate_node_key::GenerateKeyCmdCommon, import_blocks_cmd::ImportBlocksCmd,
insert_key::InsertKeyCmd, inspect_key::InspectKeyCmd, inspect_node_key::InspectNodeKeyCmd,
key::KeySubcommand, purge_chain_cmd::PurgeChainCmd, revert_cmd::RevertCmd, run_cmd::RunCmd,
sign::SignCmd, vanity::VanityCmd, verify::VerifyCmd,
diff --git a/substrate/client/cli/src/config.rs b/substrate/client/cli/src/config.rs
index 5def9ce9b72620eb7942ac6ee68b16493f6b8053..70a4885e5eef79780bdbec5b2cb7550b28ad866a 100644
--- a/substrate/client/cli/src/config.rs
+++ b/substrate/client/cli/src/config.rs
@@ -428,8 +428,10 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
/// By default this is retrieved from `NodeKeyParams` if it is available. Otherwise its
/// `NodeKeyConfig::default()`.
fn node_key(&self, net_config_dir: &PathBuf) -> Result<NodeKeyConfig> {
+ let is_dev = self.is_dev()?;
+ let role = self.role(is_dev)?;
self.node_key_params()
- .map(|x| x.node_key(net_config_dir))
+ .map(|x| x.node_key(net_config_dir, role, is_dev))
.unwrap_or_else(|| Ok(Default::default()))
}
@@ -463,11 +465,9 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
let is_dev = self.is_dev()?;
let chain_id = self.chain_id(is_dev)?;
let chain_spec = cli.load_spec(&chain_id)?;
- let base_path = self
- .base_path()?
- .unwrap_or_else(|| BasePath::from_project("", "", &C::executable_name()));
- let config_dir = base_path.config_dir(chain_spec.id());
- let net_config_dir = config_dir.join(DEFAULT_NETWORK_CONFIG_PATH);
+ let base_path = base_path_or_default(self.base_path()?, &C::executable_name());
+ let config_dir = build_config_dir(&base_path, chain_spec.id());
+ let net_config_dir = build_net_config_dir(&config_dir);
let client_id = C::client_id();
let database_cache_size = self.database_cache_size()?.unwrap_or(1024);
let database = self.database()?.unwrap_or(
@@ -665,3 +665,33 @@ pub fn generate_node_name() -> String {
}
}
}
+
+/// Returns the value of `base_path` or the default_path if it is None
+pub(crate) fn base_path_or_default(
+ base_path: Option<BasePath>,
+ executable_name: &String,
+) -> BasePath {
+ base_path.unwrap_or_else(|| BasePath::from_project("", "", executable_name))
+}
+
+/// Returns the default path for configuration directory based on the chain_spec
+pub(crate) fn build_config_dir(base_path: &BasePath, chain_spec_id: &str) -> PathBuf {
+ base_path.config_dir(chain_spec_id)
+}
+
+/// Returns the default path for the network configuration inside the configuration dir
+pub(crate) fn build_net_config_dir(config_dir: &PathBuf) -> PathBuf {
+ config_dir.join(DEFAULT_NETWORK_CONFIG_PATH)
+}
+
+/// Returns the default path for the network directory starting from the provided base_path
+/// or from the default base_path.
+pub(crate) fn build_network_key_dir_or_default(
+ base_path: Option<BasePath>,
+ chain_spec_id: &str,
+ executable_name: &String,
+) -> PathBuf {
+ let config_dir =
+ build_config_dir(&base_path_or_default(base_path, executable_name), chain_spec_id);
+ build_net_config_dir(&config_dir)
+}
diff --git a/substrate/client/cli/src/error.rs b/substrate/client/cli/src/error.rs
index 6c0cfca4932efca53ff6f8d38e8ae4de004f8b91..90ad048009ade90b5ccd4cfc264cc85e3b311843 100644
--- a/substrate/client/cli/src/error.rs
+++ b/substrate/client/cli/src/error.rs
@@ -18,6 +18,8 @@
//! Initialization errors.
+use std::path::PathBuf;
+
use sp_core::crypto;
/// Result type alias for the CLI.
@@ -78,6 +80,20 @@ pub enum Error {
#[error(transparent)]
GlobalLoggerError(#[from] sc_tracing::logging::Error),
+
+ #[error(
+ "Starting an authorithy without network key in {0}.
+ \n This is not a safe operation because other authorities in the network may depend on your node having a stable identity.
+ \n Otherwise these other authorities may not being able to reach you.
+ \n If it is the first time running your node you could use one of the following methods:
+ \n 1. [Preferred] Separately generate the key with: <NODE_BINARY> key generate-node-key --base-path <YOUR_BASE_PATH>
+ \n 2. [Preferred] Separately generate the key with: <NODE_BINARY> key generate-node-key --file <YOUR_PATH_TO_NODE_KEY>
+ \n 3. [Preferred] Separately generate the key with: <NODE_BINARY> key generate-node-key --default-base-path
+ \n 4. [Unsafe] Pass --unsafe-force-node-key-generation and make sure you remove it for subsequent node restarts"
+ )]
+ NetworkKeyNotFound(PathBuf),
+ #[error("A network key already exists in path {0}")]
+ KeyAlreadyExistsInPath(PathBuf),
}
impl From<&str> for Error {
diff --git a/substrate/client/cli/src/params/node_key_params.rs b/substrate/client/cli/src/params/node_key_params.rs
index 53f19f58e1fb0d657d387fd50a8a2346c238ead4..7058af19f1d4a5edc31a8e93d062f7f93c065f1e 100644
--- a/substrate/client/cli/src/params/node_key_params.rs
+++ b/substrate/client/cli/src/params/node_key_params.rs
@@ -18,15 +18,16 @@
use clap::Args;
use sc_network::config::{identity::ed25519, NodeKeyConfig};
+use sc_service::Role;
use sp_core::H256;
use std::{path::PathBuf, str::FromStr};
-use crate::{arg_enums::NodeKeyType, error};
+use crate::{arg_enums::NodeKeyType, error, Error};
/// The file name of the node's Ed25519 secret key inside the chain-specific
/// network config directory, if neither `--node-key` nor `--node-key-file`
/// is specified in combination with `--node-key-type=ed25519`.
-const NODE_KEY_ED25519_FILE: &str = "secret_ed25519";
+pub(crate) const NODE_KEY_ED25519_FILE: &str = "secret_ed25519";
/// Parameters used to create the `NodeKeyConfig`, which determines the keypair
/// used for libp2p networking.
@@ -79,22 +80,48 @@ pub struct NodeKeyParams {
/// the chosen type.
#[arg(long, value_name = "FILE")]
pub node_key_file: Option<PathBuf>,
+
+ /// Forces key generation if node-key-file file does not exist.
+ ///
+ /// This is an unsafe feature for production networks, because as an active authority
+ /// other authorities may depend on your node having a stable identity and they might
+ /// not being able to reach you if your identity changes after entering the active set.
+ ///
+ /// For minimal node downtime if no custom `node-key-file` argument is provided
+ /// the network-key is usually persisted accross nodes restarts,
+ /// in the `network` folder from directory provided in `--base-path`
+ ///
+ /// Warning!! If you ever run the node with this argument, make sure
+ /// you remove it for the subsequent restarts.
+ #[arg(long)]
+ pub unsafe_force_node_key_generation: bool,
}
impl NodeKeyParams {
/// Create a `NodeKeyConfig` from the given `NodeKeyParams` in the context
/// of an optional network config storage directory.
- pub fn node_key(&self, net_config_dir: &PathBuf) -> error::Result<NodeKeyConfig> {
+ pub fn node_key(
+ &self,
+ net_config_dir: &PathBuf,
+ role: Role,
+ is_dev: bool,
+ ) -> error::Result<NodeKeyConfig> {
Ok(match self.node_key_type {
NodeKeyType::Ed25519 => {
let secret = if let Some(node_key) = self.node_key.as_ref() {
parse_ed25519_secret(node_key)?
} else {
- sc_network::config::Secret::File(
- self.node_key_file
- .clone()
- .unwrap_or_else(|| net_config_dir.join(NODE_KEY_ED25519_FILE)),
- )
+ let key_path = self
+ .node_key_file
+ .clone()
+ .unwrap_or_else(|| net_config_dir.join(NODE_KEY_ED25519_FILE));
+ if !self.unsafe_force_node_key_generation &&
+ role.is_authority() && !is_dev &&
+ !key_path.exists()
+ {
+ return Err(Error::NetworkKeyNotFound(key_path))
+ }
+ sc_network::config::Secret::File(key_path)
};
NodeKeyConfig::Ed25519(secret)
@@ -122,7 +149,8 @@ mod tests {
use super::*;
use clap::ValueEnum;
use libp2p_identity::ed25519;
- use std::fs;
+ use std::fs::{self, File};
+ use tempfile::TempDir;
#[test]
fn test_node_key_config_input() {
@@ -136,8 +164,9 @@ mod tests {
node_key_type,
node_key: Some(format!("{:x}", H256::from_slice(sk.as_ref()))),
node_key_file: None,
+ unsafe_force_node_key_generation: false,
};
- params.node_key(net_config_dir).and_then(|c| match c {
+ params.node_key(net_config_dir, Role::Authority, false).and_then(|c| match c {
NodeKeyConfig::Ed25519(sc_network::config::Secret::Input(ref ski))
if node_key_type == NodeKeyType::Ed25519 && &sk[..] == ski.as_ref() =>
Ok(()),
@@ -156,10 +185,11 @@ mod tests {
node_key_type: NodeKeyType::Ed25519,
node_key: None,
node_key_file: Some(file),
+ unsafe_force_node_key_generation: false,
};
let node_key = params
- .node_key(&PathBuf::from("not-used"))
+ .node_key(&PathBuf::from("not-used"), Role::Authority, false)
.expect("Creates node key config")
.into_keypair()
.expect("Creates node key pair");
@@ -186,29 +216,58 @@ mod tests {
#[test]
fn test_node_key_config_default() {
- fn with_def_params<F>(f: F) -> error::Result<()>
+ fn with_def_params<F>(f: F, unsafe_force_node_key_generation: bool) -> error::Result<()>
where
F: Fn(NodeKeyParams) -> error::Result<()>,
{
NodeKeyType::value_variants().iter().try_for_each(|t| {
let node_key_type = *t;
- f(NodeKeyParams { node_key_type, node_key: None, node_key_file: None })
+ f(NodeKeyParams {
+ node_key_type,
+ node_key: None,
+ node_key_file: None,
+ unsafe_force_node_key_generation,
+ })
})
}
- fn some_config_dir(net_config_dir: &PathBuf) -> error::Result<()> {
- with_def_params(|params| {
- let dir = PathBuf::from(net_config_dir.clone());
- let typ = params.node_key_type;
- params.node_key(net_config_dir).and_then(move |c| match c {
- NodeKeyConfig::Ed25519(sc_network::config::Secret::File(ref f))
- if typ == NodeKeyType::Ed25519 && f == &dir.join(NODE_KEY_ED25519_FILE) =>
- Ok(()),
- _ => Err(error::Error::Input("Unexpected node key config".into())),
- })
- })
+ fn some_config_dir(
+ net_config_dir: &PathBuf,
+ unsafe_force_node_key_generation: bool,
+ role: Role,
+ is_dev: bool,
+ ) -> error::Result<()> {
+ with_def_params(
+ |params| {
+ let dir = PathBuf::from(net_config_dir.clone());
+ let typ = params.node_key_type;
+ let role = role.clone();
+ params.node_key(net_config_dir, role, is_dev).and_then(move |c| match c {
+ NodeKeyConfig::Ed25519(sc_network::config::Secret::File(ref f))
+ if typ == NodeKeyType::Ed25519 &&
+ f == &dir.join(NODE_KEY_ED25519_FILE) =>
+ Ok(()),
+ _ => Err(error::Error::Input("Unexpected node key config".into())),
+ })
+ },
+ unsafe_force_node_key_generation,
+ )
}
- assert!(some_config_dir(&PathBuf::from_str("x").unwrap()).is_ok());
+ assert!(some_config_dir(&PathBuf::from_str("x").unwrap(), false, Role::Full, false).is_ok());
+ assert!(
+ some_config_dir(&PathBuf::from_str("x").unwrap(), false, Role::Authority, true).is_ok()
+ );
+ assert!(
+ some_config_dir(&PathBuf::from_str("x").unwrap(), true, Role::Authority, false).is_ok()
+ );
+ assert!(matches!(
+ some_config_dir(&PathBuf::from_str("x").unwrap(), false, Role::Authority, false),
+ Err(Error::NetworkKeyNotFound(_))
+ ));
+
+ let tempdir = TempDir::new().unwrap();
+ let _file = File::create(tempdir.path().join(NODE_KEY_ED25519_FILE)).unwrap();
+ assert!(some_config_dir(&tempdir.path().into(), false, Role::Authority, false).is_ok());
}
}
diff --git a/substrate/client/tracing/src/logging/event_format.rs b/substrate/client/tracing/src/logging/event_format.rs
index 9589c1dfee28d11fe0dd7d409f9cec6e3563b812..8d875427841e94c933ed0a704050409c7e23552a 100644
--- a/substrate/client/tracing/src/logging/event_format.rs
+++ b/substrate/client/tracing/src/logging/event_format.rs
@@ -21,6 +21,7 @@ use ansi_term::Colour;
use regex::Regex;
use std::fmt::{self, Write};
use tracing::{Event, Level, Subscriber};
+use tracing_log::NormalizeEvent;
use tracing_subscriber::{
fmt::{format, time::FormatTime, FmtContext, FormatEvent, FormatFields},
registry::LookupSpan,
@@ -60,10 +61,12 @@ where
N: for<'a> FormatFields<'a> + 'static,
{
let mut writer = &mut ControlCodeSanitizer::new(!self.enable_color, writer);
+ let normalized_meta = event.normalized_metadata();
+ let meta = normalized_meta.as_ref().unwrap_or_else(|| event.metadata());
time::write(&self.timer, &mut format::Writer::new(&mut writer), self.enable_color)?;
if self.display_level {
- let fmt_level = FmtLevel::new(event.metadata().level(), self.enable_color);
+ let fmt_level = FmtLevel::new(meta.level(), self.enable_color);
write!(writer, "{} ", fmt_level)?;
}
@@ -81,7 +84,7 @@ where
}
if self.display_target {
- write!(writer, "{}: ", event.metadata().target())?;
+ write!(writer, "{}: ", meta.target())?;
}
// Custom code to display node name
diff --git a/substrate/frame/assets/Cargo.toml b/substrate/frame/assets/Cargo.toml
index 3b95750c14c8c9c78ad86442085580810e447f5a..ed6df77e15232d7249fdcdb5da41a673ef420994 100644
--- a/substrate/frame/assets/Cargo.toml
+++ b/substrate/frame/assets/Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "pallet-assets"
-version = "29.0.0"
+version = "29.1.0"
authors.workspace = true
edition.workspace = true
license = "Apache-2.0"
diff --git a/substrate/frame/assets/src/functions.rs b/substrate/frame/assets/src/functions.rs
index 8791aaa736b350292cfce58c4d4067f3e8af101b..4a5fb06ee2c82ecce0f936d546f317b157fb9722 100644
--- a/substrate/frame/assets/src/functions.rs
+++ b/substrate/frame/assets/src/functions.rs
@@ -491,7 +491,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
let d = Asset::<T, I>::get(&id).ok_or(Error::<T, I>::Unknown)?;
ensure!(
d.status == AssetStatus::Live || d.status == AssetStatus::Frozen,
- Error::<T, I>::AssetNotLive
+ Error::<T, I>::IncorrectStatus
);
let actual = Self::decrease_balance(id.clone(), target, amount, f, |actual, details| {
diff --git a/substrate/frame/assets/src/lib.rs b/substrate/frame/assets/src/lib.rs
index 6891f04dfb51aae356a708112a7bba284e894be2..c6b379e1d060600f8c531ef75c3663cbb667e230 100644
--- a/substrate/frame/assets/src/lib.rs
+++ b/substrate/frame/assets/src/lib.rs
@@ -987,7 +987,7 @@ pub mod pallet {
let d = Asset::<T, I>::get(&id).ok_or(Error::<T, I>::Unknown)?;
ensure!(
d.status == AssetStatus::Live || d.status == AssetStatus::Frozen,
- Error::<T, I>::AssetNotLive
+ Error::<T, I>::IncorrectStatus
);
ensure!(origin == d.freezer, Error::<T, I>::NoPermission);
let who = T::Lookup::lookup(who)?;
@@ -1024,7 +1024,7 @@ pub mod pallet {
let details = Asset::<T, I>::get(&id).ok_or(Error::<T, I>::Unknown)?;
ensure!(
details.status == AssetStatus::Live || details.status == AssetStatus::Frozen,
- Error::<T, I>::AssetNotLive
+ Error::<T, I>::IncorrectStatus
);
ensure!(origin == details.admin, Error::<T, I>::NoPermission);
let who = T::Lookup::lookup(who)?;
@@ -1113,7 +1113,7 @@ pub mod pallet {
Asset::<T, I>::try_mutate(id.clone(), |maybe_details| {
let details = maybe_details.as_mut().ok_or(Error::<T, I>::Unknown)?;
- ensure!(details.status == AssetStatus::Live, Error::<T, I>::LiveAsset);
+ ensure!(details.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);
ensure!(origin == details.owner, Error::<T, I>::NoPermission);
if details.owner == owner {
return Ok(())
@@ -1669,7 +1669,7 @@ pub mod pallet {
let d = Asset::<T, I>::get(&id).ok_or(Error::<T, I>::Unknown)?;
ensure!(
d.status == AssetStatus::Live || d.status == AssetStatus::Frozen,
- Error::<T, I>::AssetNotLive
+ Error::<T, I>::IncorrectStatus
);
ensure!(origin == d.freezer, Error::<T, I>::NoPermission);
let who = T::Lookup::lookup(who)?;
diff --git a/substrate/frame/broker/Cargo.toml b/substrate/frame/broker/Cargo.toml
index 969f13e269de4208a220150d688a21ffcc0191f1..f36b94c3cec567ebe25a2194876598e5a3b92a56 100644
--- a/substrate/frame/broker/Cargo.toml
+++ b/substrate/frame/broker/Cargo.toml
@@ -29,6 +29,7 @@ frame-system = { path = "../system", default-features = false }
[dev-dependencies]
sp-io = { path = "../../primitives/io" }
+pretty_assertions = "1.3.0"
[features]
default = ["std"]
diff --git a/substrate/frame/broker/src/mock.rs b/substrate/frame/broker/src/mock.rs
index c7205058c9720b15838de3684a33303a45cb8c68..6219b4eff1b457294c9f3656e4be145c3639e2e0 100644
--- a/substrate/frame/broker/src/mock.rs
+++ b/substrate/frame/broker/src/mock.rs
@@ -29,7 +29,7 @@ use frame_support::{
};
use frame_system::{EnsureRoot, EnsureSignedBy};
use sp_arithmetic::Perbill;
-use sp_core::{ConstU32, ConstU64};
+use sp_core::{ConstU32, ConstU64, Get};
use sp_runtime::{
traits::{BlockNumberProvider, Identity},
BuildStorage, Saturating,
@@ -210,6 +210,15 @@ pub fn advance_to(b: u64) {
}
}
+pub fn advance_sale_period() {
+ let sale = SaleInfo::<Test>::get().unwrap();
+
+ let target_block_number =
+ sale.region_begin as u64 * <<Test as crate::Config>::TimeslicePeriod as Get<u64>>::get();
+
+ advance_to(target_block_number)
+}
+
pub fn pot() -> u64 {
balance(Broker::account_id())
}
diff --git a/substrate/frame/broker/src/tests.rs b/substrate/frame/broker/src/tests.rs
index 0aacd7ef3696c600ed9546d66fc84fb851553ee6..d738d34450336e8aa8c20eb2c7d232d2c69db592 100644
--- a/substrate/frame/broker/src/tests.rs
+++ b/substrate/frame/broker/src/tests.rs
@@ -24,6 +24,7 @@ use frame_support::{
BoundedVec,
};
use frame_system::RawOrigin::Root;
+use pretty_assertions::assert_eq;
use sp_runtime::{traits::Get, TokenError};
use CoreAssignment::*;
use CoretimeTraceItem::*;
@@ -891,6 +892,161 @@ fn short_leases_are_cleaned() {
});
}
+#[test]
+fn leases_can_be_renewed() {
+ TestExt::new().endow(1, 1000).execute_with(|| {
+ // Timeslice period is 2.
+ //
+ // Sale 1 starts at block 7, Sale 2 starts at 13.
+
+ // Set lease to expire in sale 1 and start sales.
+ assert_ok!(Broker::do_set_lease(2001, 9));
+ assert_eq!(Leases::<Test>::get().len(), 1);
+ // Start the sales with only one core for this lease.
+ assert_ok!(Broker::do_start_sales(100, 1));
+
+ // Advance to sale period 1, we should get an AllowedRenewal for task 2001 for the next
+ // sale.
+ advance_sale_period();
+ assert_eq!(
+ AllowedRenewals::<Test>::get(AllowedRenewalId { core: 0, when: 10 }),
+ Some(AllowedRenewalRecord {
+ price: 100,
+ completion: CompletionStatus::Complete(
+ vec![ScheduleItem { mask: CoreMask::complete(), assignment: Task(2001) }]
+ .try_into()
+ .unwrap()
+ )
+ })
+ );
+ // And the lease has been removed from storage.
+ assert_eq!(Leases::<Test>::get().len(), 0);
+
+ // Advance to sale period 2, where we can renew.
+ advance_sale_period();
+ assert_ok!(Broker::do_renew(1, 0));
+ // We renew for the base price of the previous sale period.
+ assert_eq!(balance(1), 900);
+
+ // We just renewed for this period.
+ advance_sale_period();
+ // Now we are off core and the core is pooled.
+ advance_sale_period();
+ // Check the trace agrees.
+ assert_eq!(
+ CoretimeTrace::get(),
+ vec![
+ // Period 0 gets no assign core, but leases are on-core.
+ // Period 1:
+ (
+ 6,
+ AssignCore {
+ core: 0,
+ begin: 8,
+ assignment: vec![(CoreAssignment::Task(2001), 57600)],
+ end_hint: None,
+ },
+ ),
+ // Period 2 - expiring at the end of this period, so we called renew.
+ (
+ 12,
+ AssignCore {
+ core: 0,
+ begin: 14,
+ assignment: vec![(CoreAssignment::Task(2001), 57600)],
+ end_hint: None,
+ },
+ ),
+ // Period 3 - we get assigned a core because we called renew in period 2.
+ (
+ 18,
+ AssignCore {
+ core: 0,
+ begin: 20,
+ assignment: vec![(CoreAssignment::Task(2001), 57600)],
+ end_hint: None,
+ },
+ ),
+ // Period 4 - we don't get a core as we didn't call renew again.
+ // This core is recycled into the pool.
+ (
+ 24,
+ AssignCore {
+ core: 0,
+ begin: 26,
+ assignment: vec![(CoreAssignment::Pool, 57600)],
+ end_hint: None,
+ },
+ ),
+ ]
+ );
+ });
+}
+
+// We understand that this does not work as intended for leases that expire within `region_length`
+// timeslices after calling `start_sales`.
+#[test]
+fn short_leases_cannot_be_renewed() {
+ TestExt::new().endow(1, 1000).execute_with(|| {
+ // Timeslice period is 2.
+ //
+ // Sale 1 starts at block 7, Sale 2 starts at 13.
+
+ // Set lease to expire in sale period 0 and start sales.
+ assert_ok!(Broker::do_set_lease(2001, 3));
+ assert_eq!(Leases::<Test>::get().len(), 1);
+ // Start the sales with one core for this lease.
+ assert_ok!(Broker::do_start_sales(100, 1));
+
+ // The lease is removed.
+ assert_eq!(Leases::<Test>::get().len(), 0);
+
+ // We should have got an entry in AllowedRenewals, but we don't because rotate_sale
+ // schedules leases a period in advance. This renewal should be in the period after next
+ // because while bootstrapping our way into the sale periods, we give everything a lease for
+ // period 1, so they can renew for period 2. So we have a core until the end of period 1,
+ // but we are not marked as able to renew because we expired before sale period 1 starts.
+ //
+ // This should be fixed.
+ assert_eq!(AllowedRenewals::<Test>::get(AllowedRenewalId { core: 0, when: 10 }), None);
+ // And the lease has been removed from storage.
+ assert_eq!(Leases::<Test>::get().len(), 0);
+
+ // Advance to sale period 2, where we now cannot renew.
+ advance_to(13);
+ assert_noop!(Broker::do_renew(1, 0), Error::<Test>::NotAllowed);
+
+ // Check the trace.
+ assert_eq!(
+ CoretimeTrace::get(),
+ vec![
+ // Period 0 gets no assign core, but leases are on-core.
+ // Period 1 we get assigned a core due to the way the sales are bootstrapped.
+ (
+ 6,
+ AssignCore {
+ core: 0,
+ begin: 8,
+ assignment: vec![(CoreAssignment::Task(2001), 57600)],
+ end_hint: None,
+ },
+ ),
+ // Period 2 - we don't get a core as we couldn't renew.
+ // This core is recycled into the pool.
+ (
+ 12,
+ AssignCore {
+ core: 0,
+ begin: 14,
+ assignment: vec![(CoreAssignment::Pool, 57600)],
+ end_hint: None,
+ },
+ ),
+ ]
+ );
+ });
+}
+
#[test]
fn leases_are_limited() {
TestExt::new().execute_with(|| {
@@ -1092,3 +1248,141 @@ fn config_works() {
assert_noop!(Broker::configure(Root.into(), cfg), Error::<Test>::InvalidConfig);
});
}
+
+/// Ensure that a lease that ended before `start_sales` was called can be renewed.
+#[test]
+fn renewal_works_leases_ended_before_start_sales() {
+ TestExt::new().endow(1, 1000).execute_with(|| {
+ let config = Configuration::<Test>::get().unwrap();
+
+ // This lease is ended before `start_stales` was called.
+ assert_ok!(Broker::do_set_lease(1, 1));
+
+ // Go to some block to ensure that the lease of task 1 already ended.
+ advance_to(5);
+
+ // This lease will end three sale periods in.
+ assert_ok!(Broker::do_set_lease(
+ 2,
+ Broker::latest_timeslice_ready_to_commit(&config) + config.region_length * 3
+ ));
+
+ // This intializes the first sale and the period 0.
+ assert_ok!(Broker::do_start_sales(100, 2));
+ assert_noop!(Broker::do_renew(1, 1), Error::<Test>::Unavailable);
+ assert_noop!(Broker::do_renew(1, 0), Error::<Test>::Unavailable);
+
+ // Lease for task 1 should have been dropped.
+ assert!(Leases::<Test>::get().iter().any(|l| l.task == 2));
+
+ // This intializes the second and the period 1.
+ advance_sale_period();
+
+ // Now we can finally renew the core 0 of task 1.
+ let new_core = Broker::do_renew(1, 0).unwrap();
+ // Renewing the active lease doesn't work.
+ assert_noop!(Broker::do_renew(1, 1), Error::<Test>::SoldOut);
+ assert_eq!(balance(1), 900);
+
+ // This intializes the third sale and the period 2.
+ advance_sale_period();
+ let new_core = Broker::do_renew(1, new_core).unwrap();
+
+ // Renewing the active lease doesn't work.
+ assert_noop!(Broker::do_renew(1, 0), Error::<Test>::SoldOut);
+ assert_eq!(balance(1), 800);
+
+ // All leases should have ended
+ assert!(Leases::<Test>::get().is_empty());
+
+ // This intializes the fourth sale and the period 3.
+ advance_sale_period();
+
+ // Renew again
+ assert_eq!(0, Broker::do_renew(1, new_core).unwrap());
+ // Renew the task 2.
+ assert_eq!(1, Broker::do_renew(1, 0).unwrap());
+ assert_eq!(balance(1), 600);
+
+ // This intializes the fifth sale and the period 4.
+ advance_sale_period();
+
+ assert_eq!(
+ CoretimeTrace::get(),
+ vec![
+ (
+ 10,
+ AssignCore {
+ core: 0,
+ begin: 12,
+ assignment: vec![(Task(1), 57600)],
+ end_hint: None
+ }
+ ),
+ (
+ 10,
+ AssignCore {
+ core: 1,
+ begin: 12,
+ assignment: vec![(Task(2), 57600)],
+ end_hint: None
+ }
+ ),
+ (
+ 16,
+ AssignCore {
+ core: 0,
+ begin: 18,
+ assignment: vec![(Task(2), 57600)],
+ end_hint: None
+ }
+ ),
+ (
+ 16,
+ AssignCore {
+ core: 1,
+ begin: 18,
+ assignment: vec![(Task(1), 57600)],
+ end_hint: None
+ }
+ ),
+ (
+ 22,
+ AssignCore {
+ core: 0,
+ begin: 24,
+ assignment: vec![(Task(2), 57600)],
+ end_hint: None,
+ },
+ ),
+ (
+ 22,
+ AssignCore {
+ core: 1,
+ begin: 24,
+ assignment: vec![(Task(1), 57600)],
+ end_hint: None,
+ },
+ ),
+ (
+ 28,
+ AssignCore {
+ core: 0,
+ begin: 30,
+ assignment: vec![(Task(1), 57600)],
+ end_hint: None,
+ },
+ ),
+ (
+ 28,
+ AssignCore {
+ core: 1,
+ begin: 30,
+ assignment: vec![(Task(2), 57600)],
+ end_hint: None,
+ },
+ ),
+ ]
+ );
+ });
+}
diff --git a/substrate/frame/broker/src/tick_impls.rs b/substrate/frame/broker/src/tick_impls.rs
index 388370bce4d4b8045122c02ad758b62b4476fd15..04e9a65bf8f67b11ef66edf7268eda74e336e65d 100644
--- a/substrate/frame/broker/src/tick_impls.rs
+++ b/substrate/frame/broker/src/tick_impls.rs
@@ -216,11 +216,10 @@ impl<T: Config> Pallet<T> {
let assignment = CoreAssignment::Task(task);
let schedule = BoundedVec::truncate_from(vec![ScheduleItem { mask, assignment }]);
Workplan::<T>::insert((region_begin, first_core), &schedule);
- // Separate these to avoid missed expired leases hanging around forever.
- let expired = until < region_end;
- let expiring = until >= region_begin && expired;
- if expiring {
- // last time for this one - make it renewable.
+ // Will the lease expire at the end of the period?
+ let expire = until < region_end;
+ if expire {
+ // last time for this one - make it renewable in the next sale.
let renewal_id = AllowedRenewalId { core: first_core, when: region_end };
let record = AllowedRenewalRecord { price, completion: Complete(schedule) };
AllowedRenewals::<T>::insert(renewal_id, &record);
@@ -232,8 +231,10 @@ impl<T: Config> Pallet<T> {
});
Self::deposit_event(Event::LeaseEnding { when: region_end, task });
}
+
first_core.saturating_inc();
- !expired
+
+ !expire
});
Leases::<T>::put(&leases);
diff --git a/substrate/primitives/api/Cargo.toml b/substrate/primitives/api/Cargo.toml
index 544ba72141ebd22291c04a54f34adbd873b335aa..2f553819b1bc09b911ad3664aa555c661863a40b 100644
--- a/substrate/primitives/api/Cargo.toml
+++ b/substrate/primitives/api/Cargo.toml
@@ -68,4 +68,4 @@ std = [
disable-logging = ["log/max_level_off"]
# Do not report the documentation in the metadata.
no-metadata-docs = ["sp-api-proc-macro/no-metadata-docs"]
-frame-metadata = ["sp-api-proc-macro/frame-metadata", "sp-metadata-ir"]
+frame-metadata = ["sp-metadata-ir"]
diff --git a/substrate/primitives/api/proc-macro/Cargo.toml b/substrate/primitives/api/proc-macro/Cargo.toml
index f5406758687ae20e9ce13a2cf02959c5cccbd576..b1bc547f3e4ae82e4e4331643c0aef14e1df3451 100644
--- a/substrate/primitives/api/proc-macro/Cargo.toml
+++ b/substrate/primitives/api/proc-macro/Cargo.toml
@@ -35,4 +35,3 @@ assert_matches = "1.3.0"
default = ["std"]
std = ["blake2/std"]
no-metadata-docs = []
-frame-metadata = []
diff --git a/substrate/primitives/api/proc-macro/src/decl_runtime_apis.rs b/substrate/primitives/api/proc-macro/src/decl_runtime_apis.rs
index e34e4c0e7672164afb41702ab3bae87082df9062..cb213f2fd627b628f2b763ff7e3f132eb849bed4 100644
--- a/substrate/primitives/api/proc-macro/src/decl_runtime_apis.rs
+++ b/substrate/primitives/api/proc-macro/src/decl_runtime_apis.rs
@@ -193,10 +193,7 @@ fn generate_runtime_decls(decls: &[ItemTrait]) -> Result<TokenStream> {
get_api_version(&found_attributes).map(|v| generate_runtime_api_version(v as u32))?;
let id = generate_runtime_api_id(&decl.ident.to_string());
- #[cfg(feature = "frame-metadata")]
let metadata = crate::runtime_metadata::generate_decl_runtime_metadata(&decl);
- #[cfg(not(feature = "frame-metadata"))]
- let metadata = quote!();
let trait_api_version = get_api_version(&found_attributes)?;
diff --git a/substrate/primitives/api/proc-macro/src/impl_runtime_apis.rs b/substrate/primitives/api/proc-macro/src/impl_runtime_apis.rs
index 87a381fd7bf92bc72e30babf018b5dcb0141e571..2c423f8c28dd435599e1cc5b2d5a49807fd73abb 100644
--- a/substrate/primitives/api/proc-macro/src/impl_runtime_apis.rs
+++ b/substrate/primitives/api/proc-macro/src/impl_runtime_apis.rs
@@ -821,10 +821,7 @@ fn impl_runtime_apis_impl_inner(api_impls: &[ItemImpl]) -> Result<TokenStream> {
let wasm_interface = generate_wasm_interface(api_impls)?;
let api_impls_for_runtime_api = generate_api_impl_for_runtime_api(api_impls)?;
- #[cfg(feature = "frame-metadata")]
let runtime_metadata = crate::runtime_metadata::generate_impl_runtime_metadata(api_impls)?;
- #[cfg(not(feature = "frame-metadata"))]
- let runtime_metadata = quote!();
let impl_ = quote!(
#base_runtime_api
diff --git a/substrate/primitives/api/proc-macro/src/lib.rs b/substrate/primitives/api/proc-macro/src/lib.rs
index 06e148880e975f88e07836a4a5ca52fd5e5527b1..d34f4b7f9cf6a388438b203dbad9f1e0c68b3481 100644
--- a/substrate/primitives/api/proc-macro/src/lib.rs
+++ b/substrate/primitives/api/proc-macro/src/lib.rs
@@ -25,7 +25,6 @@ mod common;
mod decl_runtime_apis;
mod impl_runtime_apis;
mod mock_impl_runtime_apis;
-#[cfg(feature = "frame-metadata")]
mod runtime_metadata;
mod utils;
diff --git a/substrate/primitives/api/proc-macro/src/runtime_metadata.rs b/substrate/primitives/api/proc-macro/src/runtime_metadata.rs
index 41849401291e633924e6f530ffc5bc3474ba6283..9944927d557302db454e9589be681ffb26c7558e 100644
--- a/substrate/primitives/api/proc-macro/src/runtime_metadata.rs
+++ b/substrate/primitives/api/proc-macro/src/runtime_metadata.rs
@@ -164,15 +164,17 @@ pub fn generate_decl_runtime_metadata(decl: &ItemTrait) -> TokenStream2 {
let (impl_generics, _, where_clause) = generics.split_for_impl();
quote!(
- #( #attrs )*
- #[inline(always)]
- pub fn runtime_metadata #impl_generics () -> #crate_::metadata_ir::RuntimeApiMetadataIR
- #where_clause
- {
- #crate_::metadata_ir::RuntimeApiMetadataIR {
- name: #trait_name,
- methods: #crate_::vec![ #( #methods, )* ],
- docs: #docs,
+ #crate_::frame_metadata_enabled! {
+ #( #attrs )*
+ #[inline(always)]
+ pub fn runtime_metadata #impl_generics () -> #crate_::metadata_ir::RuntimeApiMetadataIR
+ #where_clause
+ {
+ #crate_::metadata_ir::RuntimeApiMetadataIR {
+ name: #trait_name,
+ methods: #crate_::vec![ #( #methods, )* ],
+ docs: #docs,
+ }
}
}
)
@@ -255,14 +257,16 @@ pub fn generate_impl_runtime_metadata(impls: &[ItemImpl]) -> Result<TokenStream2
// `construct_runtime!` is called.
Ok(quote!(
- #[doc(hidden)]
- trait InternalImplRuntimeApis {
- #[inline(always)]
- fn runtime_metadata(&self) -> #crate_::vec::Vec<#crate_::metadata_ir::RuntimeApiMetadataIR> {
- #crate_::vec![ #( #metadata, )* ]
+ #crate_::frame_metadata_enabled! {
+ #[doc(hidden)]
+ trait InternalImplRuntimeApis {
+ #[inline(always)]
+ fn runtime_metadata(&self) -> #crate_::vec::Vec<#crate_::metadata_ir::RuntimeApiMetadataIR> {
+ #crate_::vec![ #( #metadata, )* ]
+ }
}
+ #[doc(hidden)]
+ impl InternalImplRuntimeApis for #runtime_name {}
}
- #[doc(hidden)]
- impl InternalImplRuntimeApis for #runtime_name {}
))
}
diff --git a/substrate/primitives/api/proc-macro/src/utils.rs b/substrate/primitives/api/proc-macro/src/utils.rs
index a6570a98f1f78361398bd5a5a2c101af0a133988..d90b56058648ba8653992c210586238028629689 100644
--- a/substrate/primitives/api/proc-macro/src/utils.rs
+++ b/substrate/primitives/api/proc-macro/src/utils.rs
@@ -261,7 +261,6 @@ pub fn versioned_trait_name(trait_ident: &Ident, version: u64) -> Ident {
}
/// Extract the documentation from the provided attributes.
-#[cfg(feature = "frame-metadata")]
pub fn get_doc_literals(attrs: &[syn::Attribute]) -> Vec<syn::Lit> {
use quote::ToTokens;
attrs
@@ -277,7 +276,6 @@ pub fn get_doc_literals(attrs: &[syn::Attribute]) -> Vec<syn::Lit> {
}
/// Filters all attributes except the cfg ones.
-#[cfg(feature = "frame-metadata")]
pub fn filter_cfg_attributes(attrs: &[syn::Attribute]) -> Vec<syn::Attribute> {
attrs.iter().filter(|a| a.path().is_ident("cfg")).cloned().collect()
}
diff --git a/substrate/primitives/api/src/lib.rs b/substrate/primitives/api/src/lib.rs
index a945b9f21f3cff60d6a9d5e24aa07704a1dc8d31..20f989c4882e35fe06d5496f851b4adec2c6f1c0 100644
--- a/substrate/primitives/api/src/lib.rs
+++ b/substrate/primitives/api/src/lib.rs
@@ -838,3 +838,4 @@ decl_runtime_apis! {
sp_core::generate_feature_enabled_macro!(std_enabled, feature = "std", $);
sp_core::generate_feature_enabled_macro!(std_disabled, not(feature = "std"), $);
+sp_core::generate_feature_enabled_macro!(frame_metadata_enabled, feature = "frame-metadata", $);