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.

Sign up for GitHub

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

Jump to bottom

Limit bundle weight based on target domain block weight #2563

Closed
wants to merge 3 commits into from
Closed

Limit bundle weight based on target domain block weight #2563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c82e6cd
refactor DomainBundleProposer to store slot probability
ParthDesai Feb 27, 2024
82cdd90
require BundleProducerElectionApi for fetching bundle slot probability
ParthDesai Feb 27, 2024
9cbefd7
calculate domain max bundle weight
ParthDesai Feb 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/subspace-malicious-operator/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ serde_json = "1.0.111"
sp-api = { version = "4.0.0-dev", git = "https://github.com/subspace/polkadot-sdk", rev = "d6b500960579d73c43fc4ef550b703acfa61c4c8" }
sp-blockchain = { version = "4.0.0-dev", git = "https://github.com/subspace/polkadot-sdk", rev = "d6b500960579d73c43fc4ef550b703acfa61c4c8" }
sp-block-builder = { git = "https://github.com/subspace/polkadot-sdk", rev = "d6b500960579d73c43fc4ef550b703acfa61c4c8", default-features = false, version = "4.0.0-dev" }
sp-consensus = { version = "0.10.0-dev", git = "https://github.com/subspace/polkadot-sdk", rev = "d6b500960579d73c43fc4ef550b703acfa61c4c8" }
sp-consensus-subspace = { version = "0.1.0", path = "../sp-consensus-subspace" }
sp-consensus-slots = { version = "0.10.0-dev", git = "https://github.com/subspace/polkadot-sdk", rev = "d6b500960579d73c43fc4ef550b703acfa61c4c8" }
sp-core = { version = "21.0.0", git = "https://github.com/subspace/polkadot-sdk", rev = "d6b500960579d73c43fc4ef550b703acfa61c4c8" }
Expand Down
13 changes: 7 additions & 6 deletions crates/subspace-malicious-operator/src/malicious_bundle_producer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use sp_api::ProvideRuntimeApi;
use sp_block_builder::BlockBuilder;
use sp_blockchain::Info;
use sp_consensus_slots::Slot;
use sp_consensus_subspace::FarmerPublicKey;
use sp_consensus_subspace::{FarmerPublicKey, SubspaceApi};
use sp_core::crypto::UncheckedFrom;
use sp_core::Get;
use sp_domains::core_api::DomainCoreApi;
Expand Down Expand Up @@ -105,7 +105,8 @@ where
CClient: HeaderBackend<CBlock> + ProvideRuntimeApi<CBlock> + 'static,
CClient::Api: DomainsApi<CBlock, <DomainBlock as BlockT>::Header>
+ BundleProducerElectionApi<CBlock, Balance>
+ AccountNonceApi<CBlock, AccountId, Nonce>,
+ AccountNonceApi<CBlock, AccountId, Nonce>
+ SubspaceApi<CBlock, FarmerPublicKey>,
TransactionPool: sc_transaction_pool_api::TransactionPool<
Block = DomainBlock,
Hash = <DomainBlock as BlockT>::Hash,
Expand All @@ -118,7 +119,7 @@ where
consensus_keystore: KeystorePtr,
consensus_offchain_tx_pool_factory: OffchainTransactionPoolFactory<CBlock>,
domain_transaction_pool: Arc<TransactionPool>,
) -> Self {
) -> Result<Self, sp_blockchain::Error> {
let operator_keystore = KeystoreContainer::new(&KeystoreConfig::InMemory)
.expect("create in-memory keystore container must succeed")
.keystore();
Expand All @@ -128,7 +129,7 @@ where
domain_client.clone(),
consensus_client.clone(),
domain_transaction_pool,
);
)?;

let (bundle_sender, _bundle_receiver) = tracing_unbounded("domain_bundle_stream", 100);
let bundle_producer = DomainBundleProducer::new(
Expand All @@ -150,7 +151,7 @@ where
.sudo_account_id(consensus_client.info().best_hash)
.expect("Failed to get sudo account");

Self {
Ok(Self {
domain_id,
consensus_client,
consensus_keystore,
Expand All @@ -160,7 +161,7 @@ where
malicious_operator_status: MaliciousOperatorStatus::NoStatus,
sudo_acccount,
consensus_offchain_tx_pool_factory,
}
})
}

async fn handle_new_slot(
Expand Down
3 changes: 2 additions & 1 deletion crates/subspace-malicious-operator/src/malicious_domain_instance_starter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ where
consensus_keystore,
consensus_offchain_tx_pool_factory,
domain_node.transaction_pool.clone(),
);
)
.map_err(Box::new)?;

domain_node
.task_manager
Expand Down
1 change: 1 addition & 0 deletions domains/client/domain-operator/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ sp-blockchain = { version = "4.0.0-dev", git = "https://github.com/subspace/polk
sp-block-builder = { version = "4.0.0-dev", git = "https://github.com/subspace/polkadot-sdk", rev = "d6b500960579d73c43fc4ef550b703acfa61c4c8" }
sp-consensus = { version = "0.10.0-dev", git = "https://github.com/subspace/polkadot-sdk", rev = "d6b500960579d73c43fc4ef550b703acfa61c4c8" }
sp-consensus-slots = { version = "0.10.0-dev", git = "https://github.com/subspace/polkadot-sdk", rev = "d6b500960579d73c43fc4ef550b703acfa61c4c8" }
sp-consensus-subspace = { path = "../../../crates/sp-consensus-subspace" }
sp-core = { version = "21.0.0", git = "https://github.com/subspace/polkadot-sdk", rev = "d6b500960579d73c43fc4ef550b703acfa61c4c8" }
sp-domains = { version = "0.1.0", path = "../../../crates/sp-domains" }
sp-domains-fraud-proof = { version = "0.1.0", path = "../../../crates/sp-domains-fraud-proof" }
Expand Down
5 changes: 4 additions & 1 deletion domains/client/domain-operator/src/domain_bundle_producer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use sc_client_api::{AuxStore, BlockBackend};
use sp_api::ProvideRuntimeApi;
use sp_block_builder::BlockBuilder;
use sp_blockchain::HeaderBackend;
use sp_consensus_subspace::{FarmerPublicKey, SubspaceApi};
use sp_domains::core_api::DomainCoreApi;
use sp_domains::{
Bundle, BundleProducerElectionApi, DomainId, DomainsApi, OperatorId, OperatorPublicKey,
Expand Down Expand Up @@ -73,7 +74,9 @@ where
Client: HeaderBackend<Block> + BlockBackend<Block> + AuxStore + ProvideRuntimeApi<Block>,
Client::Api: BlockBuilder<Block> + DomainCoreApi<Block> + TaggedTransactionQueue<Block>,
CClient: HeaderBackend<CBlock> + ProvideRuntimeApi<CBlock>,
CClient::Api: DomainsApi<CBlock, Block::Header> + BundleProducerElectionApi<CBlock, Balance>,
CClient::Api: DomainsApi<CBlock, Block::Header>
+ BundleProducerElectionApi<CBlock, Balance>
+ SubspaceApi<CBlock, FarmerPublicKey>,
TransactionPool:
sc_transaction_pool_api::TransactionPool<Block = Block, Hash = <Block as BlockT>::Hash>,
{
Expand Down
96 changes: 77 additions & 19 deletions domains/client/domain-operator/src/domain_bundle_proposer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,15 @@ use sc_transaction_pool_api::InPoolTransaction;
use sp_api::{ApiExt, ProvideRuntimeApi};
use sp_block_builder::BlockBuilder;
use sp_blockchain::HeaderBackend;
use sp_consensus_subspace::{FarmerPublicKey, SubspaceApi};
use sp_domains::core_api::DomainCoreApi;
use sp_domains::{
BundleHeader, DomainId, DomainsApi, ExecutionReceipt, HeaderHashingFor, ProofOfElection,
BundleHeader, BundleProducerElectionApi, DomainBlockLimit, DomainId, DomainsApi,
ExecutionReceipt, HeaderHashingFor, ProofOfElection,
};
use sp_runtime::traits::{
Block as BlockT, Hash as HashT, Header as HeaderT, IntegerSquareRoot, NumberFor, One, Zero,
};
use sp_runtime::traits::{Block as BlockT, Hash as HashT, Header as HeaderT, NumberFor, One, Zero};
use sp_runtime::Percent;
use sp_transaction_pool::runtime_api::TaggedTransactionQueue;
use sp_weights::Weight;
Expand Down Expand Up @@ -58,12 +62,33 @@ impl<Block: BlockT, CBlock: BlockT> PreviousBundledTx<Block, CBlock> {
}
}

pub fn calculate_max_bundle_weight_and_size(
domain_block_limit: DomainBlockLimit,
consensus_slot_probability: (u64, u64),
bundle_slot_probability: (u64, u64),
) -> (Weight, u32) {
// (n1 / d1) / (n2 / d2) is equal to (n1 * d2) / (d1 * n2)
// This represents: bundle_slot_probability/SLOT_PROBABILITY
let expected_bundles_per_block = (bundle_slot_probability.0 * consensus_slot_probability.1)
/ (bundle_slot_probability.1 * consensus_slot_probability.0);

// This represents: Ceil[2*Sqrt[bundle_slot_probability/SLOT_PROBABILITY]])
let std_of_expected_bundles_per_block = (2 * expected_bundles_per_block.integer_sqrt()) + 1;

// max_bundle_weight = TargetDomainBlockWeight/(bundle_slot_probability/SLOT_PROBABILITY+ Ceil[2*Sqrt[ bundle_slot_probability/SLOT_PROBABILITY]])
let max_bundle_weight = domain_block_limit.max_block_weight
/ (expected_bundles_per_block + std_of_expected_bundles_per_block);

(max_bundle_weight, domain_block_limit.max_block_size)
Copy link
Member

Choose a reason for hiding this comment

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

@NingLin-P do you recall if we discussed anything about limiting bundle size too based on expected number of bundles?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I believe we also need to limit the bundle size here.

Copy link
Member

Choose a reason for hiding this comment

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

@ParthDesai let's do it the same way as weight then:

Suggested change
(max_bundle_weight, domain_block_limit.max_block_size)
let max_bundle_size = domain_block_limit.max_block_size
/ (expected_bundles_per_block + std_of_expected_bundles_per_block);
(max_bundle_weight, max_bundle_size)

}

pub struct DomainBundleProposer<Block: BlockT, Client, CBlock: BlockT, CClient, TransactionPool> {
domain_id: DomainId,
client: Arc<Client>,
consensus_client: Arc<CClient>,
transaction_pool: Arc<TransactionPool>,
previous_bundled_tx: PreviousBundledTx<Block, CBlock>,
consensus_slot_probability: (u64, u64),
_phantom_data: PhantomData<(Block, CBlock)>,
}

Expand All @@ -77,6 +102,7 @@ impl<Block: BlockT, Client, CBlock: BlockT, CClient, TransactionPool> Clone
consensus_client: self.consensus_client.clone(),
transaction_pool: self.transaction_pool.clone(),
previous_bundled_tx: PreviousBundledTx::new(),
consensus_slot_probability: self.consensus_slot_probability,
_phantom_data: self._phantom_data,
}
}
Expand All @@ -96,7 +122,9 @@ where
Client: HeaderBackend<Block> + BlockBackend<Block> + AuxStore + ProvideRuntimeApi<Block>,
Client::Api: BlockBuilder<Block> + DomainCoreApi<Block> + TaggedTransactionQueue<Block>,
CClient: HeaderBackend<CBlock> + ProvideRuntimeApi<CBlock>,
CClient::Api: DomainsApi<CBlock, Block::Header>,
CClient::Api: DomainsApi<CBlock, Block::Header>
+ SubspaceApi<CBlock, FarmerPublicKey>
+ BundleProducerElectionApi<CBlock, Balance>,
TransactionPool:
sc_transaction_pool_api::TransactionPool<Block = Block, Hash = <Block as BlockT>::Hash>,
{
Expand All @@ -105,15 +133,53 @@ where
client: Arc<Client>,
consensus_client: Arc<CClient>,
transaction_pool: Arc<TransactionPool>,
) -> Self {
Self {
) -> sp_blockchain::Result<Self> {
let chain_constants = consensus_client
.runtime_api()
.chain_constants(consensus_client.info().best_hash)
.map_err(|api_error| sp_blockchain::Error::Application(api_error.into()))?;

Ok(Self {
domain_id,
client,
consensus_client,
transaction_pool,
previous_bundled_tx: PreviousBundledTx::new(),
consensus_slot_probability: chain_constants.slot_probability(),
_phantom_data: PhantomData,
}
})
}

fn max_bundle_weight_and_size(&self) -> sp_blockchain::Result<(Weight, u32)> {
let domain_block_limit = self
.consensus_client
.runtime_api()
.domain_block_limit(self.consensus_client.info().best_hash, self.domain_id)?
.ok_or_else(|| {
sp_blockchain::Error::Application(
format!("Domain block limit for {:?} not found", self.domain_id).into(),
)
})?;
let consensus_slot_probability = self.consensus_slot_probability;
let bundle_slot_probability = self
.consensus_client
.runtime_api()
.bundle_producer_election_params(
self.consensus_client.info().best_hash,
self.domain_id,
)?
.ok_or_else(|| {
sp_blockchain::Error::Application(
format!("Domain configuration for {:?} not found", self.domain_id).into(),
)
})?
.bundle_slot_probability;

Ok(calculate_max_bundle_weight_and_size(
domain_block_limit,
consensus_slot_probability,
bundle_slot_probability,
))
}

pub(crate) async fn propose_bundle_at(
Expand Down Expand Up @@ -146,15 +212,7 @@ where
.maybe_clear(self.consensus_client.info().best_hash);

let bundle_vrf_hash = U256::from_be_bytes(proof_of_election.vrf_hash());
let domain_block_limit = self
.consensus_client
.runtime_api()
.domain_block_limit(self.consensus_client.info().best_hash, self.domain_id)?
.ok_or_else(|| {
sp_blockchain::Error::Application(
format!("Domain block limit for {:?} not found", self.domain_id).into(),
)
})?;
let (max_bundle_weight, max_bundle_size) = self.max_bundle_weight_and_size()?;
let mut extrinsics = Vec::new();
let mut estimated_bundle_weight = Weight::default();
let mut bundle_size = 0u32;
Expand Down Expand Up @@ -199,11 +257,11 @@ where
})?;
let next_estimated_bundle_weight =
estimated_bundle_weight.saturating_add(tx_weight);
if next_estimated_bundle_weight.any_gt(domain_block_limit.max_block_weight) {
if next_estimated_bundle_weight.any_gt(max_bundle_weight) {
if skipped < MAX_SKIPPED_TRANSACTIONS
&& Percent::from_rational(
estimated_bundle_weight.ref_time(),
domain_block_limit.max_block_weight.ref_time(),
max_bundle_weight.ref_time(),
) < BUNDLE_UTILIZATION_THRESHOLD
{
skipped += 1;
Expand All @@ -213,9 +271,9 @@ where
}

let next_bundle_size = bundle_size + pending_tx_data.encoded_size() as u32;
if next_bundle_size > domain_block_limit.max_block_size {
if next_bundle_size > max_bundle_size {
if skipped < MAX_SKIPPED_TRANSACTIONS
&& Percent::from_rational(bundle_size, domain_block_limit.max_block_size)
&& Percent::from_rational(bundle_size, max_bundle_size)
< BUNDLE_UTILIZATION_THRESHOLD
{
skipped += 1;
Expand Down
4 changes: 3 additions & 1 deletion domains/client/domain-operator/src/domain_worker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use sc_transaction_pool_api::OffchainTransactionPoolFactory;
use sp_api::{ApiExt, ProvideRuntimeApi};
use sp_block_builder::BlockBuilder;
use sp_blockchain::{HeaderBackend, HeaderMetadata};
use sp_consensus_subspace::{FarmerPublicKey, SubspaceApi};
use sp_core::traits::{CodeExecutor, SpawnEssentialNamed};
use sp_core::H256;
use sp_domains::core_api::DomainCoreApi;
Expand Down Expand Up @@ -92,7 +93,8 @@ pub(super) async fn start_worker<
CClient::Api: DomainsApi<CBlock, Block::Header>
+ MessengerApi<CBlock, NumberFor<CBlock>>
+ BundleProducerElectionApi<CBlock, Balance>
+ FraudProofApi<CBlock, Block::Header>,
+ FraudProofApi<CBlock, Block::Header>
+ SubspaceApi<CBlock, FarmerPublicKey>,
TransactionPool: sc_transaction_pool_api::TransactionPool<Block = Block, Hash = <Block as BlockT>::Hash>
+ 'static,
Backend: sc_client_api::Backend<Block> + 'static,
Expand Down
8 changes: 5 additions & 3 deletions domains/client/domain-operator/src/operator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use sc_client_api::{
use sc_utils::mpsc::tracing_unbounded;
use sp_api::ProvideRuntimeApi;
use sp_blockchain::{HeaderBackend, HeaderMetadata};
use sp_consensus_subspace::{FarmerPublicKey, SubspaceApi};
use sp_core::traits::{CodeExecutor, SpawnEssentialNamed};
use sp_core::H256;
use sp_domains::core_api::DomainCoreApi;
Expand Down Expand Up @@ -92,7 +93,8 @@ where
CClient::Api: DomainsApi<CBlock, Block::Header>
+ MessengerApi<CBlock, NumberFor<CBlock>>
+ BundleProducerElectionApi<CBlock, Balance>
+ FraudProofApi<CBlock, Block::Header>,
+ FraudProofApi<CBlock, Block::Header>
+ SubspaceApi<CBlock, FarmerPublicKey>,
Backend: sc_client_api::Backend<Block> + Send + Sync + 'static,
TransactionPool: sc_transaction_pool_api::TransactionPool<Block = Block, Hash = <Block as BlockT>::Hash>
+ 'static,
Expand All @@ -114,7 +116,7 @@ where
NSNS,
ASS,
>,
) -> Result<Self, sp_consensus::Error>
) -> Result<Self, sp_blockchain::Error>
where
IBNS: Stream<Item = (NumberFor<CBlock>, mpsc::Sender<()>)> + Send + 'static,
CIBNS: Stream<Item = BlockImportNotification<CBlock>> + Send + 'static,
Expand All @@ -126,7 +128,7 @@ where
params.client.clone(),
params.consensus_client.clone(),
params.transaction_pool.clone(),
);
)?;

let bundle_producer = DomainBundleProducer::new(
params.domain_id,
Expand Down
6 changes: 4 additions & 2 deletions domains/client/domain-operator/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2954,7 +2954,8 @@ async fn stale_and_in_future_bundle_should_be_rejected() {
alice.client.clone(),
ferdie.client.clone(),
alice.operator.transaction_pool.clone(),
);
)
.unwrap();
let (bundle_sender, _bundle_receiver) =
sc_utils::mpsc::tracing_unbounded("domain_bundle_stream", 100);
DomainBundleProducer::new(
Expand Down Expand Up @@ -3871,7 +3872,8 @@ async fn test_bad_receipt_chain() {
alice.client.clone(),
ferdie.client.clone(),
alice.operator.transaction_pool.clone(),
);
)
.unwrap();
let (bundle_sender, _bundle_receiver) =
sc_utils::mpsc::tracing_unbounded("domain_bundle_stream", 100);
DomainBundleProducer::new(
Expand Down
1 change: 1 addition & 0 deletions domains/service/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ sp-block-builder = { version = "4.0.0-dev", git = "https://github.com/subspace/p
sp-blockchain = { version = "4.0.0-dev", git = "https://github.com/subspace/polkadot-sdk", rev = "d6b500960579d73c43fc4ef550b703acfa61c4c8" }
sp-consensus = { version = "0.10.0-dev", git = "https://github.com/subspace/polkadot-sdk", rev = "d6b500960579d73c43fc4ef550b703acfa61c4c8" }
sp-consensus-slots = { version = "0.10.0-dev", git = "https://github.com/subspace/polkadot-sdk", rev = "d6b500960579d73c43fc4ef550b703acfa61c4c8" }
sp-consensus-subspace = { version = "0.1.0", path = "../../crates/sp-consensus-subspace" }
sp-core = { version = "21.0.0", git = "https://github.com/subspace/polkadot-sdk", rev = "d6b500960579d73c43fc4ef550b703acfa61c4c8" }
sp-domains = { version = "0.1.0", path = "../../crates/sp-domains" }
sp-domains-fraud-proof = { version = "0.1.0", path = "../../crates/sp-domains-fraud-proof" }
Expand Down
Loading
Loading