Skip to content

Commit

Permalink
breaking: remove validation of spending counter for votecast
Browse files Browse the repository at this point in the history
  • Loading branch information
saibatizoku committed Jul 25, 2023
1 parent efe089a commit ace69ec
Show file tree
Hide file tree
Showing 7 changed files with 316 additions and 76 deletions.
1 change: 1 addition & 0 deletions chain-impl-mockchain/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ rand_chacha = { version = "0.3", optional = true }
criterion = { version = "0.3.0", optional = true }
rand = "0.8"
cryptoxide = "0.4"
tracing = "0.1"

[features]
property-test-api = [
Expand Down
33 changes: 25 additions & 8 deletions chain-impl-mockchain/src/accounting/account/account_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,18 +162,35 @@ impl<Extra: Clone> AccountState<Extra> {
Ok(st)
}

/// Subtract a value from an account state, and return the new state.
/// Spends value from an account state, and returns the new state.
///
/// Note that this *also* increment the counter, as this function would be usually call
/// for spending.
pub fn sub(&self, spending: SpendingCounter, v: Value) -> Result<Option<Self>, LedgerError> {
/// Note that this *also* increments the counter, as this function is usually
/// used for spending.
pub fn spend(&self, spending: SpendingCounter, v: Value) -> Result<Option<Self>, LedgerError> {
let new_value = (self.value - v)?;
let mut r = self.clone();
r.spending.next_verify(spending)?;
r.value = new_value;
Ok(Some(r))
}

/// Spends value from an account state, and returns the new state.
///
/// Note that this *also* increments the counter, but does not fail if the
/// given counter fails to match the current one. However, it does throw
/// a warning.
pub(crate) fn spend_unchecked(
&self,
counter: SpendingCounter,
v: Value,
) -> Result<Option<Self>, LedgerError> {
let new_value = (self.value - v)?;
let mut r = self.clone();
r.spending.next_unchecked(counter);
r.value = new_value;
Ok(Some(r))
}

/// Add a value to a token in an account state
///
/// Only error if value is overflowing
Expand Down Expand Up @@ -226,7 +243,7 @@ mod tests {
account_state.spending = SpendingCounterIncreasing::new_from_counter(counter);
assert_eq!(
should_sub_fail(account_state.clone(), sub_value),
account_state.sub(counter, sub_value).is_err(),
account_state.spend(counter, sub_value).is_err(),
)
}

Expand Down Expand Up @@ -385,7 +402,7 @@ mod tests {
}
ArbitraryAccountStateOp::Sub(value) => {
let should_fail = should_sub_fail(account_state.clone(), value);
match (should_fail, account_state.sub(counter, value)) {
match (should_fail, account_state.spend(counter, value)) {
(false, Ok(account_state)) => {
strategy.next_verify(counter).expect("success");
counter = counter.increment();
Expand All @@ -399,8 +416,8 @@ mod tests {
}
}
(true, Err(_)) => account_state,
(false, Err(err)) => panic!("Operation {}: unexpected sub operation failure. Expected success but got: {:?}",op_counter,err),
(true, Ok(account_state)) => panic!("Operation {}: unexpected sub operation success. Expected failure but got: success. AccountState: {:?}",op_counter, &account_state),
(false, Err(err)) => panic!("Operation {}: unexpected spend operation failure. Expected success but got: {:?}",op_counter,err),
(true, Ok(account_state)) => panic!("Operation {}: unexpected spend operation success. Expected failure but got: success. AccountState: {:?}",op_counter, &account_state),
}
}
ArbitraryAccountStateOp::Delegate(stake_pool_id) => {
Expand Down
35 changes: 26 additions & 9 deletions chain-impl-mockchain/src/accounting/account/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,17 +176,34 @@ impl<ID: Clone + Eq + Hash, Extra: Clone> Ledger<ID, Extra> {
.map(Ledger)
}

/// Subtract value to an existing account.
/// Spend value from an existing account.
///
/// If the account doesn't exist, or that the value would become negative, errors out.
pub fn remove_value(
/// If the account doesn't exist, or if the value is too much to spend,
/// or if the spending counter doesn't match, it throws a `LedgerError`.
pub fn spend(
&self,
identifier: &ID,
spending: SpendingCounter,
counter: SpendingCounter,
value: Value,
) -> Result<Self, LedgerError> {
self.0
.update(identifier, |st| st.sub(spending, value))
.update(identifier, |st| st.spend(counter, value))
.map(Ledger)
.map_err(|e| e.into())
}

/// Spend value from an existing account without spending counter check.
///
/// If the account doesn't exist, or if the value is too much to spend,
/// it throws a `LedgerError`.
pub(crate) fn spend_with_no_counter_check(
&self,
identifier: &ID,
counter: SpendingCounter,
value: Value,
) -> Result<Self, LedgerError> {
self.0
.update(identifier, |st| st.spend_unchecked(counter, value))
.map(Ledger)
.map_err(|e| e.into())
}
Expand Down Expand Up @@ -596,7 +613,7 @@ mod tests {
}

// remove value from account
ledger = match ledger.remove_value(&account_id, spending_counter, value) {
ledger = match ledger.spend(&account_id, spending_counter, value) {
Ok(ledger) => ledger,
Err(err) => {
return TestResult::error(format!(
Expand Down Expand Up @@ -625,7 +642,7 @@ mod tests {
}

// removes all funds from account
ledger = match ledger.remove_value(&account_id, spending_counter, value_before_reward) {
ledger = match ledger.spend(&account_id, spending_counter, value_before_reward) {
Ok(ledger) => ledger,
Err(err) => {
return TestResult::error(format!(
Expand Down Expand Up @@ -677,7 +694,7 @@ mod tests {
}

#[quickcheck]
pub fn ledger_total_value_is_correct_after_remove_value(
pub fn ledger_total_value_is_correct_after_spend(
id: Identifier,
account_state: AccountState<()>,
value_to_remove: Value,
Expand All @@ -686,7 +703,7 @@ mod tests {
ledger = ledger
.add_account(id.clone(), account_state.value(), ())
.unwrap();
let result = ledger.remove_value(&id, SpendingCounter::zero(), value_to_remove);
let result = ledger.spend(&id, SpendingCounter::zero(), value_to_remove);
let expected_result = account_state.value() - value_to_remove;
match (result, expected_result) {
(Err(_), Err(_)) => verify_total_value(ledger, account_state.value()),
Expand Down
18 changes: 17 additions & 1 deletion chain-impl-mockchain/src/accounting/account/spending.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,26 @@ impl SpendingCounterIncreasing {
actual: counter,
})
} else {
self.nexts[counter.lane()] = actual_counter.increment();
self.next_unchecked(counter);
Ok(())
}
}

/// Increases the spending counter on the given lane.
pub(crate) fn next_unchecked(&mut self, unchecked_counter: SpendingCounter) {
let lane = unchecked_counter.lane();
let counter_to_update = self.nexts[lane];
if counter_to_update != unchecked_counter {
tracing::warn!(
"Invalid spending counter, {}",
LedgerError::SpendingCredentialInvalid {
expected: counter_to_update,
actual: unchecked_counter,
}
);
}
self.nexts[lane] = counter_to_update.increment();
}
}

// only used to print the account's ledger
Expand Down
4 changes: 3 additions & 1 deletion chain-impl-mockchain/src/ledger/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ pub(super) fn valid_stake_owner_delegation_transaction(
/// check that the transaction input/outputs/witnesses is valid for the ballot
///
/// * Only 1 input (subsequently 1 witness), no output
pub(super) fn valid_vote_cast(tx: &TransactionSlice<certificate::VoteCast>) -> LedgerCheck {
pub(super) fn valid_vote_cast_tx_slice(
tx: &TransactionSlice<certificate::VoteCast>,
) -> LedgerCheck {
if_cond_fail_with!(
tx.inputs().nb_inputs() != 1
|| tx.witnesses().nb_witnesses() != 1
Expand Down
Loading

0 comments on commit ace69ec

Please sign in to comment.