Skip to content

Commit

Permalink
[refactor] hyperledger-iroha#3895: move commit_topology into block …
Browse files Browse the repository at this point in the history
…payload (hyperledger-iroha#3916)

Signed-off-by: Marin Veršić <[email protected]>
  • Loading branch information
mversic authored Sep 27, 2023
1 parent df343f3 commit 97f3c8e
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 56 deletions.
2 changes: 1 addition & 1 deletion core/benches/kura.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ async fn measure_block_size_for_n_validators(n_validators: u32) {
let mut wsv = WorldStateView::new(World::new(), kura);
let topology = Topology::new(UniqueVec::new());
let mut block = BlockBuilder::new(vec![tx], topology, Vec::new())
.chain_first(&mut wsv)
.chain(0, &mut wsv)
.sign(KeyPair::generate().unwrap())
.unwrap();

Expand Down
2 changes: 1 addition & 1 deletion core/benches/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ fn sign_blocks(criterion: &mut Criterion) {
let mut success_count = 0;
let mut failures_count = 0;

let block = BlockBuilder::new(vec![transaction], topology, Vec::new()).chain_first(&mut wsv);
let block = BlockBuilder::new(vec![transaction], topology, Vec::new()).chain(0, &mut wsv);

let _ = criterion.bench_function("sign_block", |b| {
b.iter(|| match block.clone().sign(key_pair.clone()) {
Expand Down
51 changes: 14 additions & 37 deletions core/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,11 @@ mod pending {
/// the previous round, which might then be processed by the trigger system.
#[derive(Debug, Clone)]
pub struct Pending {
/// Unix timestamp
timestamp_ms: u64,
/// The topology at the time of block commit.
commit_topology: Topology,
/// Collection of transactions which have been accepted.
/// Transaction will be validated when block is chained.
transactions: Vec<AcceptedTransaction>,
/// The topology at the time of block commit.
commit_topology: Topology,
/// Event recommendations for use in triggers and off-chain work
event_recommendations: Vec<Event>,
}
Expand All @@ -136,26 +134,23 @@ mod pending {
assert!(!transactions.is_empty(), "Empty block created");

Self(Pending {
timestamp_ms: iroha_data_model::current_time()
.as_millis()
.try_into()
.expect("Time should fit into u64"),
transactions,
commit_topology,
event_recommendations,
})
}

fn make_header(
timestamp_ms: u64,
previous_height: u64,
previous_block_hash: Option<HashOf<VersionedSignedBlock>>,
view_change_index: u64,
transactions: &[TransactionValue],
commit_topology: Topology,
) -> BlockHeader {
BlockHeader {
timestamp_ms,
timestamp_ms: iroha_data_model::current_time()
.as_millis()
.try_into()
.expect("Time should fit into u64"),
consensus_estimation_ms: DEFAULT_CONSENSUS_ESTIMATION_MS,
height: previous_height + 1,
view_change_index,
Expand All @@ -165,7 +160,6 @@ mod pending {
.map(TransactionValue::hash)
.collect::<MerkleTree<_>>()
.hash(),
commit_topology: commit_topology.ordered_peers,
}
}

Expand Down Expand Up @@ -196,6 +190,8 @@ mod pending {
}

/// Chain the block with existing blockchain.
///
/// Upon executing this method current timestamp is stored in the block header.
pub fn chain(
self,
view_change_index: u64,
Expand All @@ -205,32 +201,13 @@ mod pending {

BlockBuilder(Chained(BlockPayload {
header: Self::make_header(
self.0.timestamp_ms,
wsv.height(),
wsv.latest_block_hash(),
view_change_index,
&transactions,
self.0.commit_topology,
),
transactions,
event_recommendations: self.0.event_recommendations,
}))
}

/// Create a new blockchain with current block as the first block.
pub fn chain_first(self, wsv: &mut WorldStateView) -> BlockBuilder<Chained> {
let transactions = Self::categorize_transactions(self.0.transactions, wsv);

BlockBuilder(Chained(BlockPayload {
header: Self::make_header(
self.0.timestamp_ms,
0,
None,
0,
&transactions,
self.0.commit_topology,
),
transactions,
commit_topology: self.0.commit_topology.ordered_peers,
event_recommendations: self.0.event_recommendations,
}))
}
Expand Down Expand Up @@ -297,7 +274,7 @@ mod valid {
topology: &Topology,
wsv: &mut WorldStateView,
) -> Result<ValidBlock, (VersionedSignedBlock, BlockValidationError)> {
let actual_commit_topology = &block.payload().header.commit_topology;
let actual_commit_topology = &block.payload().commit_topology;
let expected_commit_topology = &topology.ordered_peers;

if actual_commit_topology != expected_commit_topology {
Expand Down Expand Up @@ -491,9 +468,9 @@ mod valid {
view_change_index: 0,
previous_block_hash: None,
transactions_hash: None,
commit_topology: UniqueVec::new(),
},
transactions: Vec::new(),
commit_topology: UniqueVec::new(),
event_recommendations: Vec::new(),
}))
.sign(KeyPair::generate().unwrap())
Expand Down Expand Up @@ -779,7 +756,7 @@ mod tests {
let transactions = vec![tx.clone(), tx];
let topology = Topology::new(UniqueVec::new());
let valid_block = BlockBuilder::new(transactions, topology, Vec::new())
.chain_first(&mut wsv)
.chain(0, &mut wsv)
.sign(alice_keys)
.expect("Valid");

Expand Down Expand Up @@ -846,7 +823,7 @@ mod tests {
let transactions = vec![tx0, tx, tx2];
let topology = Topology::new(UniqueVec::new());
let valid_block = BlockBuilder::new(transactions, topology, Vec::new())
.chain_first(&mut wsv)
.chain(0, &mut wsv)
.sign(alice_keys)
.expect("Valid");

Expand Down Expand Up @@ -899,7 +876,7 @@ mod tests {
let transactions = vec![tx_fail, tx_accept];
let topology = Topology::new(UniqueVec::new());
let valid_block = BlockBuilder::new(transactions, topology, Vec::new())
.chain_first(&mut wsv)
.chain(0, &mut wsv)
.sign(alice_keys)
.expect("Valid");

Expand Down
4 changes: 2 additions & 2 deletions core/src/smartcontracts/isi/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ mod tests {

let topology = Topology::new(UniqueVec::new());
let first_block = BlockBuilder::new(transactions.clone(), topology.clone(), Vec::new())
.chain_first(&mut wsv)
.chain(0, &mut wsv)
.sign(ALICE_KEYS.clone())?
.commit(&topology)
.expect("Block is valid");
Expand Down Expand Up @@ -429,7 +429,7 @@ mod tests {

let topology = Topology::new(UniqueVec::new());
let vcb = BlockBuilder::new(vec![va_tx.clone()], topology.clone(), Vec::new())
.chain_first(&mut wsv)
.chain(0, &mut wsv)
.sign(ALICE_KEYS.clone())?
.commit(&topology)
.expect("Block is valid");
Expand Down
10 changes: 5 additions & 5 deletions core/src/sumeragi/main_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ impl Sumeragi {

let mut new_wsv = self.wsv.clone();
let genesis = BlockBuilder::new(transactions, self.current_topology.clone(), vec![])
.chain_first(&mut new_wsv)
.chain(0, &mut new_wsv)
.sign(self.key_pair.clone())
.expect("Genesis signing failed");

Expand Down Expand Up @@ -324,7 +324,7 @@ impl Sumeragi {
// Parameters are updated before updating public copy of sumeragi
self.update_params();

let block_topology = block.payload().header.commit_topology.clone();
let block_topology = block.payload().commit_topology.clone();
let block_signees = block
.signatures()
.into_iter()
Expand Down Expand Up @@ -1195,7 +1195,7 @@ mod tests {

// Creating a block of two identical transactions and validating it
let block = BlockBuilder::new(vec![tx.clone(), tx], topology.clone(), Vec::new())
.chain_first(&mut wsv)
.chain(0, &mut wsv)
.sign(leader_key_pair.clone())
.expect("Block is valid");

Expand Down Expand Up @@ -1247,7 +1247,7 @@ mod tests {
let wsv = finalized_wsv.clone();

// Malform block to make it invalid
block.payload_mut().header.commit_topology.clear();
block.payload_mut().commit_topology.clear();

let result = handle_block_sync(block, &wsv, &finalized_wsv);
assert!(matches!(result, Err((_, BlockSyncError::BlockNotValid(_)))))
Expand All @@ -1270,7 +1270,7 @@ mod tests {
kura.store_block(committed_block);

// Malform block to make it invalid
block.payload_mut().header.commit_topology.clear();
block.payload_mut().commit_topology.clear();

let result = handle_block_sync(block, &wsv, &finalized_wsv);
assert!(matches!(
Expand Down
3 changes: 1 addition & 2 deletions core/src/sumeragi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,7 @@ impl SumeragiHandle {
"Sumeragi could not load block that was reported as present. \
Please check that the block storage was not disconnected.",
);
let mut topology =
Topology::new(block_ref.payload().header.commit_topology.clone());
let mut topology = Topology::new(block_ref.payload().commit_topology.clone());
topology.rotate_set_a();
topology
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/sumeragi/network_topology.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ impl Topology {
view_change_index: u64,
new_peers: UniqueVec<PeerId>,
) -> Self {
let mut topology = Topology::new(block.payload().header().commit_topology.clone());
let mut topology = Topology::new(block.payload().commit_topology.clone());
let block_signees = block
.signatures()
.into_iter()
Expand Down
6 changes: 3 additions & 3 deletions data_model/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,6 @@ pub mod model {
pub previous_block_hash: Option<HashOf<VersionedSignedBlock>>,
/// Hash of merkle tree root of transactions' hashes.
pub transactions_hash: Option<HashOf<MerkleTree<VersionedSignedTransaction>>>,
/// Topology of the network at the time of block commit.
#[getset(skip)] // FIXME: Because ffi related issues
pub commit_topology: UniqueVec<peer::PeerId>,
/// Value of view change index. Used to resolve soft forks.
pub view_change_index: u64,
/// Estimation of consensus duration (in milliseconds).
Expand Down Expand Up @@ -92,6 +89,9 @@ pub mod model {
pub struct BlockPayload {
/// Block header
pub header: BlockHeader,
/// Topology of the network at the time of block commit.
#[getset(skip)] // FIXME: Because ffi related issues
pub commit_topology: UniqueVec<peer::PeerId>,
/// array of transactions, which successfully passed validation and consensus step.
#[getset(skip)] // FIXME: Because ffi related issues
pub transactions: Vec<TransactionValue>,
Expand Down
8 changes: 4 additions & 4 deletions docs/source/references/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -672,10 +672,6 @@
"name": "transactions_hash",
"type": "Option<HashOf<MerkleTree<VersionedSignedTransaction>>>"
},
{
"name": "commit_topology",
"type": "UniqueVec<PeerId>"
},
{
"name": "view_change_index",
"type": "u64"
Expand All @@ -693,6 +689,10 @@
"name": "header",
"type": "BlockHeader"
},
{
"name": "commit_topology",
"type": "UniqueVec<PeerId>"
},
{
"name": "transactions",
"type": "Vec<TransactionValue>"
Expand Down

0 comments on commit 97f3c8e

Please sign in to comment.