Skip to content

Commit

Permalink
fix: address the remaining review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
polydez committed Apr 24, 2024
1 parent 0121c8a commit 2acd244
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 79 deletions.
4 changes: 2 additions & 2 deletions miden-tx/src/verifier/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ impl TransactionVerifier {
// build stack inputs and outputs
let stack_inputs = TransactionKernel::build_input_stack(
transaction.account_id(),
transaction.account_update().init_hash(),
transaction.account_update().init_state_hash(),
transaction.input_notes().commitment(),
transaction.block_ref(),
);
let stack_outputs = TransactionKernel::build_output_stack(
transaction.account_update().final_hash(),
transaction.account_update().new_state_hash(),
transaction.output_notes().commitment(),
);

Expand Down
10 changes: 5 additions & 5 deletions objects/src/block/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pub use note_tree::{BlockNoteIndex, BlockNoteTree};

use crate::{
notes::Nullifier,
transaction::{AccountUpdateData, OutputNote},
transaction::{AccountUpdateInfo, OutputNote},
utils::{ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable},
};

Expand All @@ -22,7 +22,7 @@ pub struct Block {
header: BlockHeader,

/// Account updates for the block.
updated_accounts: Vec<AccountUpdateData>,
updated_accounts: Vec<AccountUpdateInfo>,

/// Note batches created in transactions in the block.
created_notes: Vec<NoteBatch>,
Expand All @@ -37,7 +37,7 @@ impl Block {
/// Creates a new block.
pub const fn new(
header: BlockHeader,
updated_accounts: Vec<AccountUpdateData>,
updated_accounts: Vec<AccountUpdateInfo>,
created_notes: Vec<NoteBatch>,
created_nullifiers: Vec<Nullifier>,
) -> Self {
Expand All @@ -55,7 +55,7 @@ impl Block {
}

/// Returns the account updates.
pub fn updated_accounts(&self) -> &[AccountUpdateData] {
pub fn updated_accounts(&self) -> &[AccountUpdateInfo] {
&self.updated_accounts
}

Expand Down Expand Up @@ -95,7 +95,7 @@ impl Deserializable for Block {
fn read_from<R: ByteReader>(source: &mut R) -> Result<Self, DeserializationError> {
Ok(Self {
header: BlockHeader::read_from(source)?,
updated_accounts: <Vec<AccountUpdateData>>::read_from(source)?,
updated_accounts: <Vec<AccountUpdateInfo>>::read_from(source)?,
created_notes: <Vec<NoteBatch>>::read_from(source)?,
created_nullifiers: <Vec<Nullifier>>::read_from(source)?,
})
Expand Down
93 changes: 53 additions & 40 deletions objects/src/transaction/account_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,11 @@ impl AccountUpdateDetails {
/// Account update data.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct AccountUpdate {
/// The hash of the account before the transaction was executed.
///
/// Set to `Digest::default()` for new accounts.
init_hash: Digest,
/// Account ID.
account_id: AccountId,

/// The hash of the account after the transaction was executed.
final_hash: Digest,
new_state_hash: Digest,

/// Optional account state changes used for on-chain accounts. This data is used to update an
/// on-chain account's state after a local transaction execution. For private accounts, this
Expand All @@ -42,18 +40,22 @@ pub struct AccountUpdate {

impl AccountUpdate {
/// Creates a new [AccountUpdate].
pub const fn new(init_hash: Digest, final_hash: Digest, details: AccountUpdateDetails) -> Self {
Self { init_hash, final_hash, details }
pub const fn new(
account_id: AccountId,
new_state_hash: Digest,
details: AccountUpdateDetails,
) -> Self {
Self { account_id, new_state_hash, details }
}

/// Returns the initial account state hash.
pub fn init_hash(&self) -> Digest {
self.init_hash
/// Returns the account ID.
pub fn account_id(&self) -> AccountId {
self.account_id
}

/// Returns the final account state hash.
pub fn final_hash(&self) -> Digest {
self.final_hash
pub fn new_state_hash(&self) -> Digest {
self.new_state_hash
}

/// Returns the account update details.
Expand All @@ -69,40 +71,53 @@ impl AccountUpdate {

/// Account update data.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct AccountUpdateData {
/// Account ID.
account_id: AccountId,

/// The hash of the account after the transaction was executed.
final_state_hash: Digest,
pub struct AccountUpdateInfo {
/// The hash of the account before the transaction was executed.
///
/// Set to `Digest::default()` for new accounts.
init_state_hash: Digest,

/// Account update details.
details: AccountUpdateDetails,
/// Account update information.
update: AccountUpdate,
}

impl AccountUpdateData {
/// Creates a new [AccountUpdateData].
impl AccountUpdateInfo {
/// Creates a new [AccountUpdateInfo].
pub const fn new(
account_id: AccountId,
final_state_hash: Digest,
init_state_hash: Digest,
new_state_hash: Digest,
details: AccountUpdateDetails,
) -> Self {
Self { account_id, final_state_hash, details }
Self {
init_state_hash,
update: AccountUpdate::new(account_id, new_state_hash, details),
}
}

/// Returns the account ID.
pub fn account_id(&self) -> AccountId {
self.account_id
self.update.account_id()
}

/// Returns the initial account state hash.
pub fn init_state_hash(&self) -> Digest {
self.init_state_hash
}

/// Returns the hash of the account after the transaction was executed.
pub fn final_state_hash(&self) -> Digest {
self.final_state_hash
pub fn new_state_hash(&self) -> Digest {
self.update.new_state_hash()
}

/// Returns the account update details.
pub fn details(&self) -> &AccountUpdateDetails {
&self.details
self.update.details()
}

/// Returns `true` if the account update details are for private account.
pub fn is_private(&self) -> bool {
self.update.is_private()
}
}

Expand Down Expand Up @@ -142,36 +157,34 @@ impl Deserializable for AccountUpdateDetails {

impl Serializable for AccountUpdate {
fn write_into<W: ByteWriter>(&self, target: &mut W) {
self.init_hash.write_into(target);
self.final_hash.write_into(target);
self.account_id.write_into(target);
self.new_state_hash.write_into(target);
self.details.write_into(target);
}
}

impl Deserializable for AccountUpdate {
fn read_from<R: ByteReader>(source: &mut R) -> Result<Self, DeserializationError> {
Ok(Self {
init_hash: Digest::read_from(source)?,
final_hash: Digest::read_from(source)?,
account_id: AccountId::read_from(source)?,
new_state_hash: Digest::read_from(source)?,
details: AccountUpdateDetails::read_from(source)?,
})
}
}

impl Serializable for AccountUpdateData {
impl Serializable for AccountUpdateInfo {
fn write_into<W: ByteWriter>(&self, target: &mut W) {
self.account_id.write_into(target);
self.final_state_hash.write_into(target);
self.details.write_into(target);
self.init_state_hash.write_into(target);
self.update.write_into(target);
}
}

impl Deserializable for AccountUpdateData {
impl Deserializable for AccountUpdateInfo {
fn read_from<R: ByteReader>(source: &mut R) -> Result<Self, DeserializationError> {
Ok(Self {
account_id: AccountId::read_from(source)?,
final_state_hash: Digest::read_from(source)?,
details: AccountUpdateDetails::read_from(source)?,
init_state_hash: Digest::read_from(source)?,
update: AccountUpdate::read_from(source)?,
})
}
}
2 changes: 1 addition & 1 deletion objects/src/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ mod transaction_id;
mod tx_args;
mod tx_witness;

pub use account_update::{AccountUpdate, AccountUpdateData, AccountUpdateDetails};
pub use account_update::{AccountUpdate, AccountUpdateDetails, AccountUpdateInfo};
pub use chain_mmr::ChainMmr;
pub use executed_tx::ExecutedTransaction;
pub use inputs::{InputNote, InputNotes, ToNullifier, TransactionInputs};
Expand Down
52 changes: 23 additions & 29 deletions objects/src/transaction/proven_tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use alloc::{string::ToString, vec::Vec};
use miden_verifier::ExecutionProof;

use super::{
AccountId, AccountUpdate, AccountUpdateDetails, Digest, InputNotes, Nullifier, OutputNote,
AccountId, AccountUpdateDetails, AccountUpdateInfo, Digest, InputNotes, Nullifier, OutputNote,
OutputNotes, TransactionId,
};
use crate::{
Expand All @@ -21,11 +21,8 @@ pub struct ProvenTransaction {
/// A unique identifier for the transaction, see [TransactionId] for additional details.
id: TransactionId,

/// ID of the account that the transaction was executed against.
account_id: AccountId,

/// Account update data.
account_update: AccountUpdate,
account_update: AccountUpdateInfo,

/// A list of nullifiers for all notes consumed by the transaction.
input_notes: InputNotes<Nullifier>,
Expand All @@ -51,11 +48,11 @@ impl ProvenTransaction {

/// Returns ID of the account against which this transaction was executed.
pub fn account_id(&self) -> AccountId {
self.account_id
self.account_update.account_id()
}

/// Returns the account update details.
pub fn account_update(&self) -> &AccountUpdate {
pub fn account_update(&self) -> &AccountUpdateInfo {
&self.account_update
}

Expand Down Expand Up @@ -83,45 +80,45 @@ impl ProvenTransaction {
// --------------------------------------------------------------------------------------------

fn validate(self) -> Result<Self, ProvenTransactionError> {
if self.account_id.is_on_chain() {
let is_new_account = self.account_update.init_hash() == Digest::default();
if self.account_id().is_on_chain() {
let is_new_account = self.account_update.init_state_hash() == Digest::default();
match self.account_update.details() {
AccountUpdateDetails::Private => {
return Err(ProvenTransactionError::OnChainAccountMissingDetails(
self.account_id,
self.account_id(),
))
},
AccountUpdateDetails::New(ref account) => {
if !is_new_account {
return Err(
ProvenTransactionError::ExistingOnChainAccountRequiresDeltaDetails(
self.account_id,
self.account_id(),
),
);
}
if account.id() != self.account_id {
if account.id() != self.account_id() {
return Err(ProvenTransactionError::AccountIdMismatch(
self.account_id,
self.account_id(),
account.id(),
));
}
if account.hash() != self.account_update.final_hash() {
if account.hash() != self.account_update.new_state_hash() {
return Err(ProvenTransactionError::AccountFinalHashMismatch(
self.account_update.final_hash(),
self.account_update.new_state_hash(),
account.hash(),
));
}
},
AccountUpdateDetails::Delta(_) => {
if is_new_account {
return Err(ProvenTransactionError::NewOnChainAccountRequiresFullDetails(
self.account_id,
self.account_id(),
));
}
},
}
} else if !self.account_update.is_private() {
return Err(ProvenTransactionError::OffChainAccountWithDetails(self.account_id));
return Err(ProvenTransactionError::OffChainAccountWithDetails(self.account_id()));
}

Ok(self)
Expand Down Expand Up @@ -217,11 +214,6 @@ impl ProvenTransactionBuilder {
/// An error will be returned if an on-chain account is used without provided on-chain detail.
/// Or if the account details, i.e. account id and final hash, don't match the transaction.
pub fn build(self) -> Result<ProvenTransaction, ProvenTransactionError> {
let account_update = AccountUpdate::new(
self.initial_account_hash,
self.final_account_hash,
self.account_update_details,
);
let input_notes =
InputNotes::new(self.input_notes).map_err(ProvenTransactionError::InputNotesError)?;
let output_notes = OutputNotes::new(self.output_notes)
Expand All @@ -232,10 +224,15 @@ impl ProvenTransactionBuilder {
input_notes.commitment(),
output_notes.commitment(),
);
let account_update = AccountUpdateInfo::new(
self.account_id,
self.initial_account_hash,
self.final_account_hash,
self.account_update_details,
);

let proven_transaction = ProvenTransaction {
id,
account_id: self.account_id,
account_update,
input_notes,
output_notes,
Expand All @@ -252,7 +249,6 @@ impl ProvenTransactionBuilder {

impl Serializable for ProvenTransaction {
fn write_into<W: ByteWriter>(&self, target: &mut W) {
self.account_id.write_into(target);
self.account_update.write_into(target);
self.input_notes.write_into(target);
self.output_notes.write_into(target);
Expand All @@ -263,8 +259,7 @@ impl Serializable for ProvenTransaction {

impl Deserializable for ProvenTransaction {
fn read_from<R: ByteReader>(source: &mut R) -> Result<Self, DeserializationError> {
let account_id = AccountId::read_from(source)?;
let account_update = AccountUpdate::read_from(source)?;
let account_update = AccountUpdateInfo::read_from(source)?;

let input_notes = InputNotes::<Nullifier>::read_from(source)?;
let output_notes = OutputNotes::read_from(source)?;
Expand All @@ -273,15 +268,14 @@ impl Deserializable for ProvenTransaction {
let proof = ExecutionProof::read_from(source)?;

let id = TransactionId::new(
account_update.init_hash(),
account_update.final_hash(),
account_update.init_state_hash(),
account_update.new_state_hash(),
input_notes.commitment(),
output_notes.commitment(),
);

let proven_transaction = Self {
id,
account_id,
account_update,
input_notes,
output_notes,
Expand Down
Loading

0 comments on commit 2acd244

Please sign in to comment.