Skip to content

Commit b7779e0

Browse files
minikinminikin
authored
Refactor SpendingCounter, replace Vec usage with array (#174)
# Description Refactor SpendingCounter implementation. Replace Vec with an array usage. At this place we are certanly know what is the size of SpendingCountersIncresing lanes, so it is better replace dynamic sized collection with the static sized collection. ## Type of change - [x] Refactoring ## How Has This Been Tested? Been fixed only existing tests, no any new tests been added.
2 parents 367cf88 + bd7d50c commit b7779e0

File tree

17 files changed

+187
-125
lines changed

17 files changed

+187
-125
lines changed

‎src/chain-libs/chain-impl-mockchain/src/account.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ use chain_core::{
88
};
99
use chain_crypto::{Ed25519, PublicKey, Signature};
1010

11-
pub use account::{DelegationRatio, DelegationType, LedgerError, SpendingCounter};
11+
pub use account::{
12+
DelegationRatio, DelegationType, LedgerError, SpendingCounter, SpendingCounterIncreasing,
13+
};
1214

1315
pub type AccountAlg = Ed25519;
1416

‎src/chain-libs/chain-impl-mockchain/src/accounting/account/spending.rs‎

Lines changed: 44 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,16 @@ pub enum Error {
66
expected: SpendingCounter,
77
actual: SpendingCounter,
88
},
9+
#[error("Invalid lane value during the SpendingCountersIncreasingInitialization, expected: {0}, got: {1}")]
10+
InvalidLaneValue(usize, usize),
911
#[error("Invalid lane: {0} or counter: {1}, expected lane < (1 << LANES_BITS), counter < (1 << UNLANES_BITS)")]
1012
InvalidLaneOrCounter(usize, u32),
1113
}
1214

1315
/// Simple strategy to spend from multiple increasing counters
1416
#[derive(Debug, Clone, PartialEq, Eq)]
1517
pub struct SpendingCounterIncreasing {
16-
nexts: Vec<SpendingCounter>,
18+
nexts: [SpendingCounter; Self::LANES],
1719
}
1820

1921
// SpendingCounterIncreasing has extra invariants (e.g. nexts has 8 elements, each belongs to a different lane), so a derived implementation is not suitable
@@ -33,25 +35,23 @@ impl SpendingCounterIncreasing {
3335
x
3436
}
3537

36-
pub fn new_from_counters(set: Vec<SpendingCounter>) -> Option<Self> {
37-
if set.len() == Self::LANES {
38-
for (i, i_set_value) in set.iter().enumerate() {
39-
if i_set_value.lane() != i {
40-
return None;
41-
}
38+
pub fn new_from_counters(
39+
nexts: [SpendingCounter; SpendingCounterIncreasing::LANES],
40+
) -> Result<Self, Error> {
41+
for (i, i_set_value) in nexts.iter().enumerate() {
42+
if i_set_value.lane() != i {
43+
return Err(Error::InvalidLaneValue(i, i_set_value.lane()));
4244
}
43-
Some(SpendingCounterIncreasing { nexts: set })
44-
} else {
45-
None
4645
}
46+
Ok(SpendingCounterIncreasing { nexts })
4747
}
4848

4949
pub fn get_valid_counter(&self) -> SpendingCounter {
5050
self.nexts[0]
5151
}
5252

53-
pub fn get_valid_counters(&self) -> Vec<SpendingCounter> {
54-
self.nexts.clone()
53+
pub fn get_valid_counters(&self) -> [SpendingCounter; Self::LANES] {
54+
self.nexts
5555
}
5656

5757
/// try to match the lane of the counter in argument, if it doesn't match
@@ -85,10 +85,16 @@ impl std::fmt::Display for SpendingCounterIncreasing {
8585

8686
impl Default for SpendingCounterIncreasing {
8787
fn default() -> Self {
88-
let mut nexts = Vec::new();
89-
for i in 0..Self::LANES {
90-
nexts.push(SpendingCounter::new(i, 0).unwrap());
91-
}
88+
let nexts = [
89+
SpendingCounter::new(0, 0).unwrap(),
90+
SpendingCounter::new(1, 0).unwrap(),
91+
SpendingCounter::new(2, 0).unwrap(),
92+
SpendingCounter::new(3, 0).unwrap(),
93+
SpendingCounter::new(4, 0).unwrap(),
94+
SpendingCounter::new(5, 0).unwrap(),
95+
SpendingCounter::new(6, 0).unwrap(),
96+
SpendingCounter::new(7, 0).unwrap(),
97+
];
9298
SpendingCounterIncreasing { nexts }
9399
}
94100
}
@@ -107,7 +113,7 @@ impl Default for SpendingCounterIncreasing {
107113
any(test, feature = "property-test-api"),
108114
derive(test_strategy::Arbitrary)
109115
)]
110-
pub struct SpendingCounter(pub(crate) u32);
116+
pub struct SpendingCounter(u32);
111117

112118
impl SpendingCounter {
113119
// on 32 bits: 0x1fff_ffff;
@@ -254,25 +260,32 @@ mod tests {
254260

255261
#[test]
256262
pub fn spending_counters_duplication() {
257-
let counters = vec![SpendingCounter::zero(), SpendingCounter::zero()];
258-
assert!(SpendingCounterIncreasing::new_from_counters(counters).is_none());
263+
let counters = [
264+
SpendingCounter::zero(),
265+
SpendingCounter::zero(),
266+
SpendingCounter::new(2, 0).unwrap(),
267+
SpendingCounter::new(3, 0).unwrap(),
268+
SpendingCounter::new(4, 0).unwrap(),
269+
SpendingCounter::new(5, 0).unwrap(),
270+
SpendingCounter::new(6, 0).unwrap(),
271+
SpendingCounter::new(7, 0).unwrap(),
272+
];
273+
assert!(SpendingCounterIncreasing::new_from_counters(counters).is_err());
259274
}
260275

261276
#[test]
262277
pub fn spending_counters_incorrect_order() {
263-
let counters = vec![
278+
let counters = [
264279
SpendingCounter::new(1, 0).unwrap(),
265280
SpendingCounter::new(0, 0).unwrap(),
281+
SpendingCounter::new(2, 0).unwrap(),
282+
SpendingCounter::new(3, 0).unwrap(),
283+
SpendingCounter::new(4, 0).unwrap(),
284+
SpendingCounter::new(5, 0).unwrap(),
285+
SpendingCounter::new(6, 0).unwrap(),
286+
SpendingCounter::new(7, 0).unwrap(),
266287
];
267-
assert!(SpendingCounterIncreasing::new_from_counters(counters).is_none());
268-
}
269-
270-
#[test]
271-
pub fn spending_counters_too_many_sub_counters() {
272-
let counters = std::iter::from_fn(|| Some(SpendingCounter::zero()))
273-
.take(SpendingCounterIncreasing::LANES + 1)
274-
.collect();
275-
assert!(SpendingCounterIncreasing::new_from_counters(counters).is_none());
288+
assert!(SpendingCounterIncreasing::new_from_counters(counters).is_err());
276289
}
277290

278291
#[quickcheck_macros::quickcheck]
@@ -315,8 +328,8 @@ mod tests {
315328
any::<SpendingCounter>()
316329
.prop_map(|counter| Some(Self::new_from_counter(counter)))
317330
.boxed(),
318-
any::<Vec<SpendingCounter>>()
319-
.prop_map(Self::new_from_counters)
331+
any::<[SpendingCounter; SpendingCounterIncreasing::LANES]>()
332+
.prop_map(|counters| Self::new_from_counters(counters).ok())
320333
.boxed(),
321334
]
322335
.prop_filter_map("must be valid spending counter set", |i| i)

‎src/chain-libs/chain-impl-mockchain/src/ledger/recovery.rs‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@
3838
use super::pots;
3939
use super::{Entry, EntryOwned};
4040
use crate::accounting::account::{
41-
AccountState, DelegationRatio, DelegationType, LastRewards, SpendingCounter,
42-
SpendingCounterIncreasing,
41+
AccountState, DelegationRatio, DelegationType, LastRewards, SpendingCounterIncreasing,
4342
};
4443
use crate::certificate::{
4544
PoolId, PoolRegistration, Proposal, Proposals, UpdateProposal, UpdateProposalId, UpdateVoterId,
@@ -161,19 +160,18 @@ fn pack_spending_strategy<W: std::io::Write>(
161160
fn unpack_spending_strategy(
162161
codec: &mut Codec<&[u8]>,
163162
) -> Result<SpendingCounterIncreasing, ReadError> {
164-
let mut counters = Vec::new();
165-
for _ in 0..SpendingCounterIncreasing::LANES {
166-
let counter = SpendingCounter(codec.get_be_u32()?);
167-
counters.push(counter);
168-
}
169-
let got_length = counters.len();
170-
SpendingCounterIncreasing::new_from_counters(counters).ok_or_else(|| {
171-
ReadError::InvalidData(format!(
172-
"wrong numbers of lanes, expecting {} but got {}",
173-
SpendingCounterIncreasing::LANES,
174-
got_length,
175-
))
176-
})
163+
let counters = [
164+
codec.get_be_u32()?.into(),
165+
codec.get_be_u32()?.into(),
166+
codec.get_be_u32()?.into(),
167+
codec.get_be_u32()?.into(),
168+
codec.get_be_u32()?.into(),
169+
codec.get_be_u32()?.into(),
170+
codec.get_be_u32()?.into(),
171+
codec.get_be_u32()?.into(),
172+
];
173+
SpendingCounterIncreasing::new_from_counters(counters)
174+
.map_err(|e| ReadError::InvalidData(e.to_string()))
177175
}
178176

179177
#[cfg(feature = "evm")]

‎src/chain-wallet-libs/bindings/wallet-core/src/c/mod.rs‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ pub mod time;
88
pub mod vote;
99

1010
use crate::{Error, Proposal, Result, Wallet};
11-
use chain_impl_mockchain::{fragment::Fragment, value::Value, vote::Choice};
11+
use chain_impl_mockchain::{
12+
account::SpendingCounterIncreasing, fragment::Fragment, value::Value, vote::Choice,
13+
};
1214
use std::convert::TryInto;
1315

1416
use thiserror::Error;
@@ -166,13 +168,11 @@ pub unsafe fn wallet_spending_counters(
166168
let wallet = non_null!(wallet);
167169
let spending_counters_out = non_null_mut!(spending_counters_ptr_out);
168170

169-
let counters = wallet.spending_counter().into_boxed_slice();
170-
let len = counters.len();
171+
let counters: Box<[u32]> = wallet.spending_counter().as_slice().into();
171172

172173
let raw_ptr = Box::into_raw(counters);
173174

174175
spending_counters_out.data = raw_ptr as *mut u32;
175-
spending_counters_out.len = len;
176176

177177
Result::success()
178178
}
@@ -215,7 +215,6 @@ pub unsafe fn wallet_total_value(wallet: WalletPtr, total_out: *mut u64) -> Resu
215215
#[repr(C)]
216216
pub struct SpendingCounters {
217217
pub data: *mut u32,
218-
pub len: usize,
219218
}
220219

221220
/// update the wallet account state
@@ -245,7 +244,13 @@ pub unsafe fn wallet_set_state(wallet: WalletPtr, value: u64, nonces: SpendingCo
245244

246245
let value = Value(value);
247246

248-
let nonces = std::slice::from_raw_parts_mut(nonces.data, nonces.len).to_vec();
247+
let nonces = match std::slice::from_raw_parts_mut(nonces.data, SpendingCounterIncreasing::LANES)
248+
.try_into()
249+
.map_err(|_e| Error::invalid_spending_counters())
250+
{
251+
Ok(nonces) => nonces,
252+
Err(e) => return e.into(),
253+
};
249254

250255
match wallet.set_state(value, nonces) {
251256
Ok(_) => Result::success(),
@@ -423,9 +428,9 @@ pub unsafe fn wallet_delete_proposal(proposal: ProposalPtr) {
423428
/// This function is safe as long as the structure returned by this library is not modified.
424429
/// This function should only be called with a structure returned by this library.
425430
pub unsafe fn spending_counters_delete(spending_counters: SpendingCounters) {
426-
let SpendingCounters { data, len } = spending_counters;
431+
let SpendingCounters { data } = spending_counters;
427432
if !data.is_null() {
428-
let data = std::slice::from_raw_parts_mut(data, len);
433+
let data = std::slice::from_raw_parts_mut(data, SpendingCounterIncreasing::LANES);
429434
let data = Box::from_raw(data as *mut [u32]);
430435
std::mem::drop(data);
431436
}

‎src/chain-wallet-libs/bindings/wallet-core/src/wallet.rs‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use crate::{Error, Proposal};
22
use chain_core::property::Serialize as _;
33
use chain_crypto::SecretKey;
44
use chain_impl_mockchain::{
5-
account::SpendingCounter,
5+
account::SpendingCounterIncreasing,
66
block::BlockDate,
77
certificate::Certificate,
88
fragment::{Fragment, FragmentId},
@@ -62,12 +62,18 @@ impl Wallet {
6262

6363
/// get the current spending counter
6464
///
65-
pub fn spending_counter(&self) -> Vec<u32> {
66-
self.account
67-
.spending_counter()
68-
.into_iter()
69-
.map(SpendingCounter::into)
70-
.collect()
65+
pub fn spending_counter(&self) -> [u32; SpendingCounterIncreasing::LANES] {
66+
let spending_counters = self.account.spending_counter();
67+
[
68+
spending_counters[0].into(),
69+
spending_counters[1].into(),
70+
spending_counters[2].into(),
71+
spending_counters[3].into(),
72+
spending_counters[4].into(),
73+
spending_counters[5].into(),
74+
spending_counters[6].into(),
75+
spending_counters[7].into(),
76+
]
7177
}
7278

7379
/// get the total value in the wallet
@@ -92,11 +98,24 @@ impl Wallet {
9298
/// before doing any transactions, otherwise future transactions may fail
9399
/// to be accepted by the blockchain nodes because of an invalid witness
94100
/// signature.
95-
pub fn set_state(&mut self, value: Value, counters: Vec<u32>) -> Result<(), Error> {
101+
pub fn set_state(
102+
&mut self,
103+
value: Value,
104+
counters: [u32; SpendingCounterIncreasing::LANES],
105+
) -> Result<(), Error> {
96106
self.account
97107
.set_state(
98108
value,
99-
counters.into_iter().map(SpendingCounter::from).collect(),
109+
[
110+
counters[0].into(),
111+
counters[1].into(),
112+
counters[2].into(),
113+
counters[3].into(),
114+
counters[4].into(),
115+
counters[5].into(),
116+
counters[6].into(),
117+
counters[7].into(),
118+
],
100119
)
101120
.map_err(|_| Error::invalid_spending_counters())
102121
}

‎src/chain-wallet-libs/wallet/src/account.rs‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use super::transaction::AccountWitnessBuilder;
22
use crate::scheme::{on_tx_input_and_witnesses, on_tx_output};
33
use crate::states::States;
44
use chain_crypto::{Ed25519, Ed25519Extended, PublicKey, SecretKey};
5-
use chain_impl_mockchain::accounting::account::SpendingCounterIncreasing;
5+
use chain_impl_mockchain::accounting::account::{spending, SpendingCounterIncreasing};
66
use chain_impl_mockchain::{
77
account::SpendingCounter,
88
fragment::{Fragment, FragmentId},
@@ -37,10 +37,10 @@ pub enum Error {
3737
NotEnoughFunds { current: Value, needed: Value },
3838
#[error("invalid lane for spending counter")]
3939
InvalidLane,
40-
#[error("malformed spending counters")]
41-
MalformedSpendingCounters,
4240
#[error("spending counter does not match current state")]
4341
NonMonotonicSpendingCounter,
42+
#[error(transparent)]
43+
SpendingCounters(#[from] spending::Error),
4444
}
4545

4646
pub enum EitherAccount {
@@ -109,16 +109,19 @@ impl Wallet {
109109
///
110110
/// TODO: change to a constructor/initializator?, or just make it so it resets the state
111111
///
112-
pub fn set_state(&mut self, value: Value, counters: Vec<SpendingCounter>) -> Result<(), Error> {
113-
let counters = SpendingCounterIncreasing::new_from_counters(counters)
114-
.ok_or(Error::MalformedSpendingCounters)?;
112+
pub fn set_state(
113+
&mut self,
114+
value: Value,
115+
counters: [SpendingCounter; SpendingCounterIncreasing::LANES],
116+
) -> Result<(), Error> {
117+
let counters = SpendingCounterIncreasing::new_from_counters(counters)?;
115118

116119
self.state = States::new(FragmentId::zero_hash(), State { value, counters });
117120

118121
Ok(())
119122
}
120123

121-
pub fn spending_counter(&self) -> Vec<SpendingCounter> {
124+
pub fn spending_counter(&self) -> [SpendingCounter; SpendingCounterIncreasing::LANES] {
122125
self.state
123126
.last_state()
124127
.state()

‎src/chain-wallet-libs/wallet/tests/account.rs‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ use self::utils::State;
44
use chain_crypto::SecretKey;
55
use chain_impl_mockchain::{
66
account::SpendingCounter,
7-
accounting::account::SpendingCounterIncreasing,
87
certificate::VoteCast,
98
fragment::Fragment,
109
value::Value,
@@ -30,9 +29,16 @@ fn update_state_overrides_old() {
3029
account
3130
.set_state(
3231
Value(110),
33-
(0..SpendingCounterIncreasing::LANES)
34-
.map(|lane| SpendingCounter::new(lane, 1).unwrap())
35-
.collect(),
32+
[
33+
SpendingCounter::new(0, 1).unwrap(),
34+
SpendingCounter::new(1, 1).unwrap(),
35+
SpendingCounter::new(2, 1).unwrap(),
36+
SpendingCounter::new(3, 1).unwrap(),
37+
SpendingCounter::new(4, 1).unwrap(),
38+
SpendingCounter::new(5, 1).unwrap(),
39+
SpendingCounter::new(6, 1).unwrap(),
40+
SpendingCounter::new(7, 1).unwrap(),
41+
],
3642
)
3743
.unwrap();
3844

0 commit comments

Comments
 (0)