Skip to content
This repository has been archived by the owner on Aug 2, 2024. It is now read-only.

Commit

Permalink
fix(transactions): remove nonce field from InvokeV0 tx (#1185)
Browse files Browse the repository at this point in the history
  • Loading branch information
tdelabro authored Oct 13, 2023
1 parent 85cb726 commit d9ec4a1
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 37 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Next release

- fix(transactions): remove `nonce` field from InvokeV0 tx
- feat(transactions): don't enforce ordering in validate_unsigned for invokeV0
- test(pallet): add function to get braavos hash
- fix: event commitment documentation typo
- ci: added testing key generation in the ci
Expand Down
57 changes: 32 additions & 25 deletions crates/pallets/starknet/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -743,24 +743,27 @@ pub mod pallet {
let sender_nonce: Felt252Wrapper = Pallet::<T>::nonce(sender_address).into();
let transaction_nonce = transaction.nonce();

// Reject transaction with an already used Nonce
if sender_nonce > *transaction_nonce {
Err(InvalidTransaction::Stale)?;
}

// A transaction with a nonce higher than the expected nonce is placed in
// the future queue of the transaction pool.
if sender_nonce < *transaction_nonce {
log!(
info,
"Nonce is too high. Expected: {:?}, got: {:?}. This transaction will be placed in the \
transaction pool and executed in the future when the nonce is reached.",
sender_nonce,
transaction_nonce
);
}
// InvokeV0 does not have a nonce
if let Some(transaction_nonce) = transaction_nonce {
// Reject transaction with an already used Nonce
if sender_nonce > *transaction_nonce {
Err(InvalidTransaction::Stale)?;
}

// A transaction with a nonce higher than the expected nonce is placed in
// the future queue of the transaction pool.
if sender_nonce < *transaction_nonce {
log!(
info,
"Nonce is too high. Expected: {:?}, got: {:?}. This transaction will be placed in the \
transaction pool and executed in the future when the nonce is reached.",
sender_nonce,
transaction_nonce
);
}
};

(transaction.sender_address(), sender_nonce, *transaction_nonce)
(transaction.sender_address(), sender_nonce, transaction_nonce.cloned())
} else {
// TODO: create and check L1 messages Nonce
unimplemented!()
Expand Down Expand Up @@ -789,20 +792,24 @@ pub mod pallet {
})?;
}

let nonce_for_priority: u64 =
transaction_nonce.try_into().map_err(|_| InvalidTransaction::Custom(NONCE_DECODE_FAILURE))?;
let nonce_for_priority: u64 = transaction_nonce
.unwrap_or(Felt252Wrapper::ZERO)
.try_into()
.map_err(|_| InvalidTransaction::Custom(NONCE_DECODE_FAILURE))?;

let mut valid_transaction_builder = ValidTransaction::with_tag_prefix("starknet")
.priority(u64::MAX - nonce_for_priority)
.and_provides((sender_address, transaction_nonce))
.longevity(T::TransactionLongevity::get())
.propagate(true);

// Enforce waiting for the tx with the previous nonce,
// to be either executed or ordered before in the block
if transaction_nonce > sender_nonce {
valid_transaction_builder = valid_transaction_builder
.and_requires((sender_address, Felt252Wrapper(transaction_nonce.0 - FieldElement::ONE)));
if let Some(transaction_nonce) = transaction_nonce {
valid_transaction_builder = valid_transaction_builder.and_provides((sender_address, transaction_nonce));
// Enforce waiting for the tx with the previous nonce,
// to be either executed or ordered before in the block
if transaction_nonce > sender_nonce {
valid_transaction_builder = valid_transaction_builder
.and_requires((sender_address, Felt252Wrapper(transaction_nonce.0 - FieldElement::ONE)));
}
}

valid_transaction_builder.build()
Expand Down
2 changes: 0 additions & 2 deletions crates/primitives/transactions/src/compute_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ impl ComputeTransactionHash for InvokeTransactionV0 {
let calldata_hash = compute_hash_on_elements(convert_calldata(&self.calldata));
let max_fee = FieldElement::from(self.max_fee);
let chain_id = chain_id.into();
let nonce = FieldElement::from(self.nonce);

H::compute_hash_on_elements(&[
prefix,
Expand All @@ -45,7 +44,6 @@ impl ComputeTransactionHash for InvokeTransactionV0 {
calldata_hash,
max_fee,
chain_id,
nonce,
])
.into()
}
Expand Down
3 changes: 1 addition & 2 deletions crates/primitives/transactions/src/compute_hash_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,13 @@ fn test_declare_v2_tx_hash() {
fn test_invoke_tx_v0_hash() {
// Computed with `calculate_transaction_hash_common` from the cairo lang package
let expected_tx_hash =
Felt252Wrapper::from_hex_be("0x054f8e66281306dd43fb035e1bf8b1f7baad8f28390f6de1f337e6be5490f1f7").unwrap();
Felt252Wrapper::from_hex_be("0x0006a8aca140749156148fa84f432f7f7b7318c119d97dd1808848fc74d1a8a6").unwrap();

let chain_id = Felt252Wrapper(FieldElement::from_byte_slice_be(b"SN_GOERLI").unwrap());

let transaction = InvokeTransactionV0 {
max_fee: 1,
signature: vec![],
nonce: Felt252Wrapper::ZERO,
contract_address: Default::default(),
entry_point_selector: Default::default(),
calldata: vec![Felt252Wrapper::ONE, Felt252Wrapper::TWO, Felt252Wrapper::THREE],
Expand Down
12 changes: 6 additions & 6 deletions crates/primitives/transactions/src/getters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ impl UserTransaction {
}
}

pub fn nonce(&self) -> &Felt252Wrapper {
pub fn nonce(&self) -> Option<&Felt252Wrapper> {
match self {
UserTransaction::Declare(tx, _) => tx.nonce(),
UserTransaction::DeployAccount(tx) => tx.nonce(),
UserTransaction::Declare(tx, _) => Some(tx.nonce()),
UserTransaction::DeployAccount(tx) => Some(tx.nonce()),
UserTransaction::Invoke(tx) => tx.nonce(),
}
}
Expand Down Expand Up @@ -182,10 +182,10 @@ impl InvokeTransaction {
}
}

pub fn nonce(&self) -> &Felt252Wrapper {
pub fn nonce(&self) -> Option<&Felt252Wrapper> {
match self {
InvokeTransaction::V0(tx) => &tx.nonce,
InvokeTransaction::V1(tx) => &tx.nonce,
InvokeTransaction::V0(_) => None,
InvokeTransaction::V1(tx) => Some(&tx.nonce),
}
}

Expand Down
1 change: 0 additions & 1 deletion crates/primitives/transactions/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ pub enum InvokeTransaction {
pub struct InvokeTransactionV0 {
pub max_fee: u128,
pub signature: Vec<Felt252Wrapper>,
pub nonce: Felt252Wrapper,
pub contract_address: Felt252Wrapper,
pub entry_point_selector: Felt252Wrapper,
pub calldata: Vec<Felt252Wrapper>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ pub fn to_starknet_core_tx<H: HasherT>(
super::InvokeTransaction::V0(super::InvokeTransactionV0 {
max_fee,
signature,
nonce: _,
contract_address,
entry_point_selector,
calldata,
Expand Down

0 comments on commit d9ec4a1

Please sign in to comment.