Skip to content

Commit

Permalink
fix review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
  • Loading branch information
mversic committed Nov 30, 2021
1 parent 1da2f49 commit baaa78d
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 36 deletions.
6 changes: 4 additions & 2 deletions core/src/kura.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl<W: WorldTrait, IO: DiskIO> Actor for KuraWithIO<W, IO> {
}
Err(error) => {
iroha_logger::error!(%error, "Initialization of kura failed");
panic!("Init failed");
panic!("Kura initialization failed");
}
}
}
Expand Down Expand Up @@ -245,7 +245,9 @@ impl<W: WorldTrait, IO: DiskIO> KuraWithIO<W, IO> {
match self.block_store.write(&block).await {
Ok(hash) => {
self.merkle_tree = self.merkle_tree.add(hash);
self.wsv.apply(block).await;
if let Err(error) = self.wsv.apply(block).await {
iroha_logger::warn!(%error, "Failed to execute transaction on WSV");
}
self.broker.issue_send(UpdateNetworkTopology).await;
self.broker.issue_send(ContinueSync).await;
Ok(hash)
Expand Down
2 changes: 1 addition & 1 deletion core/src/smartcontracts/isi/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ mod tests {
.sign(ALICE_KEYS.clone())
.expect("Failed to sign blocks.")
.commit();
wsv.apply(vcb).await;
wsv.apply(vcb).await?;

let wrong_hash = Hash::new(&[2_u8]);
let not_found = FindTransactionByHash::new(wrong_hash).execute(&wsv);
Expand Down
3 changes: 3 additions & 0 deletions core/src/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,16 +313,19 @@ impl AcceptedTransaction {
}

impl IsInBlockchain for VersionedAcceptedTransaction {
#[inline]
fn is_in_blockchain<W: WorldTrait>(&self, wsv: &WorldStateView<W>) -> bool {
wsv.has_transaction(&self.hash())
}
}
impl IsInBlockchain for VersionedValidTransaction {
#[inline]
fn is_in_blockchain<W: WorldTrait>(&self, wsv: &WorldStateView<W>) -> bool {
wsv.has_transaction(&self.hash())
}
}
impl IsInBlockchain for VersionedRejectedTransaction {
#[inline]
fn is_in_blockchain<W: WorldTrait>(&self, wsv: &WorldStateView<W>) -> bool {
wsv.has_transaction(&self.hash())
}
Expand Down
53 changes: 29 additions & 24 deletions core/src/wsv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,15 @@ pub struct WorldStateView<W: WorldTrait> {
}

impl<W: WorldTrait + Default> Default for WorldStateView<W> {
#[inline]
fn default() -> Self {
Self::new(W::default())
}
}

impl World {
/// Creates an empty `World`.
#[inline]
pub fn new() -> Self {
Self::default()
}
Expand Down Expand Up @@ -121,7 +123,11 @@ impl<W: WorldTrait> WorldStateView<W> {
#[iroha_futures::telemetry_future]
pub async fn init(&self, blocks: Vec<VersionedCommittedBlock>) {
for block in blocks {
self.apply(block).await
#[allow(clippy::panic)]
if let Err(error) = self.apply(block).await {
iroha_logger::error!(%error, "Initialization of WSV failed");
panic!("WSV initialization failed");
}
}
}

Expand Down Expand Up @@ -151,39 +157,36 @@ impl<W: WorldTrait> WorldStateView<W> {
account.permission_tokens.clone()
}

/// Apply instructions to the `WorldStateView<W>`.
/// Apply `CommittedBlock` with changes in form of **Iroha Special Instructions** to `self`.
///
/// # Errors
/// Can fail if execution of instructions fail(should be fine after validation)
fn proceed(&self, tx: &VersionedValidTransaction) -> Result<()> {
let tx = tx.as_inner_v1();

for instruction in &tx.payload.instructions {
instruction
.clone()
.execute(tx.payload.account_id.clone(), self)?;
}
// XXX: Should it just return `()`?
Ok(())
}

/// Apply `CommittedBlock` with changes in form of **Iroha Special Instructions** to `self`.
/// Can fail if execution of instruction fails(should be fine after validation)
#[iroha_futures::telemetry_future]
#[iroha_logger::log(skip(self, block))]
pub async fn apply(&self, block: VersionedCommittedBlock) {
pub async fn apply(&self, block: VersionedCommittedBlock) -> Result<()> {
for tx in &block.as_inner_v1().transactions {
if let Err(error) = self.proceed(tx) {
iroha_logger::warn!(%error, "Failed to proceed transaction on WSV");
}
let account_id = &tx.payload().account_id;
tx.as_inner_v1()
.payload
.instructions
.iter()
.cloned()
.try_for_each(|instruction| instruction.execute(account_id.clone(), self))?;

self.transactions.insert(tx.hash());
// Yeild control cooperatively to the task scheduler.
// The transaction processing is a long CPU intensive task, so this should be included here.
task::yield_now().await;
}
for tx in &block.as_inner_v1().rejected_transactions {
self.transactions.insert(tx.hash());
}
block
.as_inner_v1()
.rejected_transactions
.iter()
.for_each(|tx| {
self.transactions.insert(tx.hash());
});
self.blocks.push(block);
Ok(())
}

/// Hash of latest block
Expand Down Expand Up @@ -500,12 +503,14 @@ impl<W: WorldTrait> WorldStateView<W> {

impl Deref for World {
type Target = World;
#[inline]
fn deref(&self) -> &Self::Target {
self
}
}

impl DerefMut for World {
#[inline]
fn deref_mut(&mut self) -> &mut Self::Target {
self
}
Expand Down Expand Up @@ -573,7 +578,7 @@ mod tests {
}
let block: VersionedCommittedBlock = block.clone().into();
block_hashes.push(block.hash());
wsv.apply(block).await;
wsv.apply(block).await.unwrap();
}

assert_eq!(
Expand Down
2 changes: 1 addition & 1 deletion core/test_network/tests/sumeragi_with_mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ pub mod utils {

async fn handle(&mut self, StoreBlock(block): StoreBlock) -> Self::Result {
self.broker.issue_send(Stored(block.hash())).await;
self.wsv.apply(block).await;
self.wsv.apply(block).await.unwrap();
self.broker.issue_send(UpdateNetworkTopology).await;
self.broker.issue_send(ContinueSync).await;
}
Expand Down
27 changes: 19 additions & 8 deletions data_model/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2029,8 +2029,10 @@ pub mod transaction {
&& self.metadata == other.metadata
}

/// Checks if number of instructions in payload exceeds maximum
///
/// # Errors
/// Asserts specific instruction number of instruction constraint
/// Fails if instruction length exceeds maximum instruction number
pub fn check_instruction_len(&self, max_instruction_number: u64) -> Result<()> {
if self
.instructions
Expand Down Expand Up @@ -2155,6 +2157,7 @@ pub mod transaction {

impl VersionedValidTransaction {
/// Same as [`as_v1`](`VersionedValidTransaction::as_v1()`) but also does conversion
#[inline]
pub const fn as_inner_v1(&self) -> &ValidTransaction {
match self {
Self::V1(v1) => &v1.0,
Expand All @@ -2180,8 +2183,10 @@ pub mod transaction {
self.as_inner_v1().hash().transmute()
}

/// Checks if number of instructions in payload exceeds maximum
///
/// # Errors
/// Asserts specific instruction number of instruction in transaction constraint
/// Fails if instruction length exceeds maximum instruction number
pub fn check_instruction_len(&self, max_instruction_len: u64) -> Result<()> {
self.as_inner_v1()
.check_instruction_len(max_instruction_len)
Expand All @@ -2201,15 +2206,17 @@ pub mod transaction {
)]
#[derive(Clone, Debug, Io, Encode, Decode, IntoSchema)]
pub struct ValidTransaction {
/// [`ValidTransaction`]'s payload.
/// The [`Transaction`]'s payload.
pub payload: Payload,
/// [`ValidTransaction`]'s [`Signature`]s.
/// [`Transaction`]'s [`Signature`]s.
pub signatures: SignaturesOf<Payload>,
}

impl ValidTransaction {
/// Checks if number of instructions in payload exceeds maximum
///
/// # Errors
/// Asserts specific instruction number of instruction in transaction constraint
/// Fails if instruction length exceeds maximum instruction number
pub fn check_instruction_len(&self, max_instruction_len: u64) -> Result<()> {
self.payload.check_instruction_len(max_instruction_len)
}
Expand Down Expand Up @@ -2255,8 +2262,10 @@ pub mod transaction {
}
}

/// Checks if number of instructions in payload exceeds maximum
///
/// # Errors
/// Asserts specific instruction number of instruction in transaction constraint
/// Fails if instruction length exceeds maximum instruction number
pub fn check_instruction_len(&self, max_instruction_len: u64) -> Result<()> {
self.as_inner_v1()
.check_instruction_len(max_instruction_len)
Expand Down Expand Up @@ -2317,7 +2326,7 @@ pub mod transaction {
Clone, Debug, Io, Encode, Decode, Serialize, Deserialize, Eq, PartialEq, IntoSchema,
)]
pub struct RejectedTransaction {
/// [`Transaction`]'s payload.
/// The [`Transaction`]'s payload.
pub payload: Payload,
/// [`Transaction`]'s [`Signature`]s.
pub signatures: SignaturesOf<Payload>,
Expand All @@ -2326,8 +2335,10 @@ pub mod transaction {
}

impl RejectedTransaction {
/// Checks if number of instructions in payload exceeds maximum
///
/// # Errors
/// Asserts specific instruction number of instruction in transaction constraint
/// Fails if instruction length exceeds maximum instruction number
pub fn check_instruction_len(&self, max_instruction_len: u64) -> Result<()> {
self.payload.check_instruction_len(max_instruction_len)
}
Expand Down

0 comments on commit baaa78d

Please sign in to comment.