Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove balance from client test and added object ownership checks in test #48

Merged
merged 9 commits into from
Dec 17, 2021
2 changes: 1 addition & 1 deletion fastpay/src/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ impl ClientServerBenchmark {
info!("Received {} responses.", responses.len(),);
} else {
// Use actual client core
let mut client = network::Client::new(
let client = network::Client::new(
self.protocol,
self.host.clone(),
self.port,
Expand Down
6 changes: 3 additions & 3 deletions fastpay/src/network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ impl Client {
}

async fn send_recv_bytes_internal(
&mut self,
&self,
shard: ShardId,
buf: Vec<u8>,
) -> Result<Vec<u8>, io::Error> {
Expand All @@ -188,7 +188,7 @@ impl Client {
}

pub async fn send_recv_bytes(
&mut self,
&self,
shard: ShardId,
buf: Vec<u8>,
) -> Result<AccountInfoResponse, FastPayError> {
Expand Down Expand Up @@ -233,7 +233,7 @@ impl AuthorityClient for Client {

/// Handle information requests for this account.
fn handle_account_info_request(
&mut self,
&self,
request: AccountInfoRequest,
) -> AsyncResult<'_, AccountInfoResponse, FastPayError> {
Box::pin(async move {
Expand Down
25 changes: 13 additions & 12 deletions fastpay_core/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub trait AuthorityClient {

/// Handle information requests for this account.
fn handle_account_info_request(
&mut self,
&self,
request: AccountInfoRequest,
) -> AsyncResult<'_, AccountInfoResponse, FastPayError>;
}
Expand Down Expand Up @@ -90,7 +90,6 @@ pub trait Client {
/// Do not confirm the transaction.
fn transfer_to_fastpay_unsafe_unconfirmed(
&mut self,
amount: Amount,
recipient: FastPayAddress,
object_id: ObjectID,
user_data: UserData,
Expand Down Expand Up @@ -253,14 +252,14 @@ where
/// Find the highest sequence number that is known to a quorum of authorities.
/// NOTE: This is only reliable in the synchronous model, with a sufficient timeout value.
#[cfg(test)]
async fn get_strong_majority_sequence_number(&mut self, object_id: ObjectID) -> SequenceNumber {
async fn get_strong_majority_sequence_number(&self, object_id: ObjectID) -> SequenceNumber {
let request = AccountInfoRequest {
object_id,
request_sequence_number: None,
request_received_transfers_excluding_first_nth: None,
};
let numbers: futures::stream::FuturesUnordered<_> = self
.authority_clients
let mut authority_clients = self.authority_clients.clone();
let numbers: futures::stream::FuturesUnordered<_> = authority_clients
.iter_mut()
.map(|(name, client)| {
let fut = client.handle_account_info_request(request.clone());
Expand All @@ -277,23 +276,26 @@ where
)
}

/// Find the highest balance that is backed by a quorum of authorities.
/// Return owner address and sequence number of an object backed by a quorum of authorities.
/// NOTE: This is only reliable in the synchronous model, with a sufficient timeout value.
#[cfg(test)]
async fn get_strong_majority_balance(&mut self, object_id: ObjectID) -> Balance {
async fn get_strong_majority_owner(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: doc comment needs updating to reflect new return value

&self,
object_id: ObjectID,
) -> Option<(FastPayAddress, SequenceNumber)> {
let request = AccountInfoRequest {
object_id,
request_sequence_number: None,
request_received_transfers_excluding_first_nth: None,
};
let numbers: futures::stream::FuturesUnordered<_> = self
.authority_clients
.iter_mut()
let authority_clients = self.authority_clients.clone();
let numbers: futures::stream::FuturesUnordered<_> = authority_clients
.iter()
.map(|(name, client)| {
let fut = client.handle_account_info_request(request.clone());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fishy that handle_account_info_request requires a mutable client, and I'd love a bit of insight as to why.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is originated from send_recv_bytes_internal in network, which required &mut self unnecessarily, I have changed it to &self in my next check in

async move {
match fut.await {
Ok(_info) => Some((*name, Balance::from(0))),
Ok(info) => Some((*name, Some((info.owner, info.next_sequence_number)))),
_ => None,
}
}
Expand Down Expand Up @@ -686,7 +688,6 @@ where

fn transfer_to_fastpay_unsafe_unconfirmed(
&mut self,
_amount: Amount,
recipient: FastPayAddress,
object_id: ObjectID,
user_data: UserData,
Expand Down
Loading