Skip to content

Commit b04aa4a

Browse files
committed
Check for sufficient balances at voting time
1 parent 8b5b9ad commit b04aa4a

File tree

14 files changed

+408
-222
lines changed

14 files changed

+408
-222
lines changed
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
// Copyright (c) Mysten Labs, Inc.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
use std::{collections::BTreeMap, sync::Arc};
5+
6+
use sui_types::{
7+
accumulator_root::{AccumulatorObjId, AccumulatorValue, U128},
8+
base_types::SequenceNumber,
9+
error::{SuiErrorKind, SuiResult, UserInputError},
10+
storage::ChildObjectResolver,
11+
};
12+
13+
pub(crate) trait AccountBalanceRead: Send + Sync {
14+
fn get_account_balance(
15+
&self,
16+
account_id: &AccumulatorObjId,
17+
// Version of the accumulator root object, used to
18+
// bound the version when we look for child account objects.
19+
accumulator_version: SequenceNumber,
20+
) -> u128;
21+
22+
/// Gets latest balance, without a version bound on the accumulator root object.
23+
/// Only used for signing time checks / RPC reads, not scheduling.
24+
fn get_latest_account_balance(&self, account_id: &AccumulatorObjId) -> u128;
25+
26+
/// Checks if balances are available in the latest versions of the referenced acccumulator
27+
/// objects. This does un-sequenced reads and can only be used on the signing/voting path
28+
/// where deterministic results are not required.
29+
fn check_balances_available(
30+
&self,
31+
requested_balances: &BTreeMap<AccumulatorObjId, u64>,
32+
) -> SuiResult {
33+
for (object_id, requested_balance) in requested_balances {
34+
let actual_balance = self.get_latest_account_balance(object_id);
35+
36+
if actual_balance < *requested_balance as u128 {
37+
return Err(SuiErrorKind::UserInputError {
38+
error: UserInputError::InvalidWithdrawReservation {
39+
error: format!(
40+
"Available balance for object id {} is less than requested: {} < {}",
41+
object_id, actual_balance, requested_balance
42+
),
43+
},
44+
}
45+
.into());
46+
}
47+
}
48+
49+
Ok(())
50+
}
51+
}
52+
53+
impl AccountBalanceRead for Arc<dyn ChildObjectResolver + Send + Sync> {
54+
fn get_account_balance(
55+
&self,
56+
account_id: &AccumulatorObjId,
57+
accumulator_version: SequenceNumber,
58+
) -> u128 {
59+
// TODO: The implementation currently relies on the fact that we could
60+
// load older versions of child objects. This has two problems:
61+
// 1. Aggressive pruning might prune old versions of child objects,
62+
// 2. Tidehunter might not continue to support this kinds of reads.
63+
// To fix this, we could also read the latest version of the accumulator root object,
64+
// and see if the provided accumulator version is already settled.
65+
let value: U128 =
66+
AccumulatorValue::load_by_id(self.as_ref(), Some(accumulator_version), *account_id)
67+
// Expect is safe because at this point we should know that we are dealing with a Balance<T>
68+
// object
69+
.expect("read cannot fail")
70+
.unwrap_or(U128 { value: 0 });
71+
72+
value.value
73+
}
74+
75+
fn get_latest_account_balance(&self, account_id: &AccumulatorObjId) -> u128 {
76+
let value = AccumulatorValue::load_by_id(self.as_ref(), None, *account_id)
77+
.expect("read cannot fail")
78+
.unwrap_or(U128 { value: 0 });
79+
80+
value.value
81+
}
82+
}

crates/sui-core/src/accumulators/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ use sui_types::{
2929

3030
use crate::execution_cache::TransactionCacheRead;
3131

32+
pub mod balance_read;
33+
3234
/// Merged value is the value stored inside accumulator objects.
3335
/// Each mergeable Move type will map to a single variant as its representation.
3436
///

crates/sui-core/src/authority.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Copyright (c) Mysten Labs, Inc.
33
// SPDX-License-Identifier: Apache-2.0
44

5+
use crate::accumulators::balance_read::AccountBalanceRead;
56
use crate::checkpoints::CheckpointBuilderError;
67
use crate::checkpoints::CheckpointBuilderResult;
78
use crate::congestion_tracker::CongestionTracker;
@@ -1024,6 +1025,12 @@ impl AuthorityState {
10241025
self.get_backing_package_store().as_ref(),
10251026
)?;
10261027

1028+
let withdraws = tx_data.process_funds_withdrawals_for_signing()?;
1029+
1030+
self.execution_cache_trait_pointers
1031+
.child_object_resolver
1032+
.check_balances_available(&withdraws)?;
1033+
10271034
let (input_objects, receiving_objects) = self.input_loader.read_objects_for_signing(
10281035
Some(tx_digest),
10291036
&input_object_kinds,

crates/sui-core/src/execution_scheduler/balance_withdraw_scheduler/eager_scheduler.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@ use sui_types::{
1414
use tokio::sync::oneshot::Sender;
1515
use tracing::{debug, instrument};
1616

17-
use crate::execution_scheduler::balance_withdraw_scheduler::{
18-
BalanceSettlement, ScheduleResult, ScheduleStatus, TxBalanceWithdraw,
19-
balance_read::AccountBalanceRead,
20-
scheduler::{BalanceWithdrawSchedulerTrait, WithdrawReservations},
17+
use crate::{
18+
accumulators::balance_read::AccountBalanceRead,
19+
execution_scheduler::balance_withdraw_scheduler::{
20+
BalanceSettlement, ScheduleResult, ScheduleStatus, TxBalanceWithdraw,
21+
scheduler::{BalanceWithdrawSchedulerTrait, WithdrawReservations},
22+
},
2123
};
2224

2325
pub(crate) struct EagerBalanceWithdrawScheduler {

crates/sui-core/src/execution_scheduler/balance_withdraw_scheduler/balance_read.rs renamed to crates/sui-core/src/execution_scheduler/balance_withdraw_scheduler/mock_balance_read.rs

Lines changed: 14 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,67 +1,26 @@
11
// Copyright (c) Mysten Labs, Inc.
22
// SPDX-License-Identifier: Apache-2.0
33

4-
#[cfg(test)]
54
use std::collections::BTreeMap;
65
use std::sync::Arc;
76

8-
#[cfg(test)]
97
use parking_lot::RwLock;
10-
#[cfg(test)]
118
use sui_types::base_types::ObjectID;
12-
use sui_types::{
13-
accumulator_root::{AccumulatorObjId, AccumulatorValue, U128},
14-
base_types::SequenceNumber,
15-
storage::ChildObjectResolver,
16-
};
9+
use sui_types::{accumulator_root::AccumulatorObjId, base_types::SequenceNumber};
1710

18-
pub(crate) trait AccountBalanceRead: Send + Sync {
19-
fn get_account_balance(
20-
&self,
21-
account_id: &AccumulatorObjId,
22-
// Version of the accumulator root object, used to
23-
// bound the version when we look for child account objects.
24-
accumulator_version: SequenceNumber,
25-
) -> u128;
26-
}
27-
28-
impl AccountBalanceRead for Arc<dyn ChildObjectResolver + Send + Sync> {
29-
fn get_account_balance(
30-
&self,
31-
account_id: &AccumulatorObjId,
32-
accumulator_version: SequenceNumber,
33-
) -> u128 {
34-
// TODO: The implementation currently relies on the fact that we could
35-
// load older versions of child objects. This has two problems:
36-
// 1. Aggressive pruning might prune old versions of child objects,
37-
// 2. Tidehunter might not continue to support this kinds of reads.
38-
// To fix this, we could also read the latest version of the accumulator root object,
39-
// and see if the provided accumulator version is already settled.
40-
let value: U128 =
41-
AccumulatorValue::load_by_id(self.as_ref(), Some(accumulator_version), *account_id)
42-
// Expect is safe because at this point we should know that we are dealing with a Balance<T>
43-
// object
44-
.expect("read cannot fail")
45-
.unwrap_or(U128 { value: 0 });
46-
47-
value.value
48-
}
49-
}
11+
use crate::accumulators::balance_read::AccountBalanceRead;
5012

5113
// Mock implementation of a balance account book for testing.
5214
// Allows setting the balance for a given account at different accumulator versions.
53-
#[cfg(test)]
5415
pub(crate) struct MockBalanceRead {
5516
inner: Arc<RwLock<MockBalanceReadInner>>,
5617
}
5718

58-
#[cfg(test)]
5919
struct MockBalanceReadInner {
6020
cur_version: SequenceNumber,
6121
balances: BTreeMap<AccumulatorObjId, BTreeMap<SequenceNumber, Option<u128>>>,
6222
}
6323

64-
#[cfg(test)]
6524
impl MockBalanceRead {
6625
pub(crate) fn new(
6726
init_version: SequenceNumber,
@@ -99,7 +58,6 @@ impl MockBalanceRead {
9958
}
10059
}
10160

102-
#[cfg(test)]
10361
impl MockBalanceReadInner {
10462
fn settle_balance_changes(
10563
&mut self,
@@ -144,9 +102,13 @@ impl MockBalanceReadInner {
144102
.last()
145103
.and_then(|(_, balance)| *balance)
146104
}
105+
106+
fn get_latest_account_balance(&self, account_id: &AccumulatorObjId) -> Option<u128> {
107+
let account_balances = self.balances.get(account_id)?;
108+
account_balances.values().last().and_then(|b| *b)
109+
}
147110
}
148111

149-
#[cfg(test)]
150112
impl AccountBalanceRead for MockBalanceRead {
151113
/// Mimic the behavior of child object read.
152114
/// Find the balance for the given account at the max version
@@ -161,4 +123,11 @@ impl AccountBalanceRead for MockBalanceRead {
161123
.get_account_balance(account_id, accumulator_version)
162124
.unwrap_or_default()
163125
}
126+
127+
fn get_latest_account_balance(&self, account_id: &AccumulatorObjId) -> u128 {
128+
let inner = self.inner.read();
129+
inner
130+
.get_latest_account_balance(account_id)
131+
.unwrap_or_default()
132+
}
164133
}

crates/sui-core/src/execution_scheduler/balance_withdraw_scheduler/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ use sui_types::{
77
accumulator_root::AccumulatorObjId, base_types::SequenceNumber, digests::TransactionDigest,
88
};
99

10-
mod balance_read;
10+
#[cfg(test)]
11+
pub(crate) mod mock_balance_read;
12+
1113
mod eager_scheduler;
1214
mod naive_scheduler;
1315
pub(crate) mod scheduler;

crates/sui-core/src/execution_scheduler/balance_withdraw_scheduler/naive_scheduler.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@ use sui_types::base_types::SequenceNumber;
77
use tokio::sync::watch;
88
use tracing::{debug, instrument};
99

10-
use crate::execution_scheduler::balance_withdraw_scheduler::{
11-
BalanceSettlement, ScheduleResult, ScheduleStatus,
12-
balance_read::AccountBalanceRead,
13-
scheduler::{BalanceWithdrawSchedulerTrait, WithdrawReservations},
10+
use crate::{
11+
accumulators::balance_read::AccountBalanceRead,
12+
execution_scheduler::balance_withdraw_scheduler::{
13+
BalanceSettlement, ScheduleResult, ScheduleStatus,
14+
scheduler::{BalanceWithdrawSchedulerTrait, WithdrawReservations},
15+
},
1416
};
1517

1618
/// A naive implementation of the balance withdraw scheduler that does not attempt to optimize the scheduling.

crates/sui-core/src/execution_scheduler/balance_withdraw_scheduler/scheduler.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@
33

44
use std::{collections::BTreeMap, sync::Arc};
55

6-
use crate::execution_scheduler::balance_withdraw_scheduler::{
7-
BalanceSettlement, ScheduleResult, ScheduleStatus, TxBalanceWithdraw,
8-
balance_read::AccountBalanceRead, eager_scheduler::EagerBalanceWithdrawScheduler,
9-
naive_scheduler::NaiveBalanceWithdrawScheduler,
6+
use crate::{
7+
accumulators::balance_read::AccountBalanceRead,
8+
execution_scheduler::balance_withdraw_scheduler::{
9+
BalanceSettlement, ScheduleResult, ScheduleStatus, TxBalanceWithdraw,
10+
eager_scheduler::EagerBalanceWithdrawScheduler,
11+
naive_scheduler::NaiveBalanceWithdrawScheduler,
12+
},
1013
};
1114
use futures::stream::FuturesUnordered;
1215
use mysten_metrics::monitored_mpsc::{UnboundedReceiver, UnboundedSender, unbounded_channel};

crates/sui-core/src/execution_scheduler/balance_withdraw_scheduler/tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
use crate::execution_scheduler::balance_withdraw_scheduler::ScheduleResult;
55
use crate::execution_scheduler::balance_withdraw_scheduler::{
6-
BalanceSettlement, ScheduleStatus, TxBalanceWithdraw, balance_read::MockBalanceRead,
6+
BalanceSettlement, ScheduleStatus, TxBalanceWithdraw, mock_balance_read::MockBalanceRead,
77
scheduler::BalanceWithdrawScheduler,
88
};
99
use futures::StreamExt;

crates/sui-core/src/execution_scheduler/execution_scheduler_impl.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ impl ExecutionScheduler {
316316
for (cert, env) in &certs {
317317
let tx_withdraws = cert
318318
.transaction_data()
319-
.process_funds_withdrawals()
319+
.process_funds_withdrawals_for_signing()
320320
.expect("Balance withdraws should have already been checked");
321321
assert!(!tx_withdraws.is_empty());
322322
let accumulator_version = env

0 commit comments

Comments
 (0)