Skip to content

Commit

Permalink
Fix certificate update issue which cause subsequence object transfer …
Browse files Browse the repository at this point in the history
…fail (#170)

* combine received cert and sent cert

* fix bugs preventing transfer of object after the first transaction, cause by incorrect certificate update.

added extensive test to check the certificate update logic.

* fix clippy

* use proper error propagation instead of unwrap

* use iterator in certificates function instead of Vec

some code clean up

* address PR comment: changed certificates in UserAccount to BTreeMap<TransactionDigest, CertifiedOrder>

* address PR comment : replaced incorrect unwrap with ?
  • Loading branch information
patrickkuo authored Jan 18, 2022
1 parent b2e5e7b commit 5a946cd
Show file tree
Hide file tree
Showing 5 changed files with 255 additions and 184 deletions.
3 changes: 1 addition & 2 deletions fastpay/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ fn make_client_state(
account.key.copy(),
committee,
authority_clients,
account.sent_certificates.clone(),
account.received_certificates.clone(),
account.certificates.clone(),
account.object_ids.clone(),
)
}
Expand Down
21 changes: 7 additions & 14 deletions fastpay/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,7 @@ pub struct UserAccount {
pub key: KeyPair,
pub object_ids: BTreeMap<ObjectID, SequenceNumber>,
pub gas_object_ids: BTreeSet<ObjectID>, // Every id in gas_object_ids should also be in object_ids.
pub sent_certificates: Vec<CertifiedOrder>,
pub received_certificates: Vec<CertifiedOrder>,
pub certificates: BTreeMap<TransactionDigest, CertifiedOrder>,
}

impl UserAccount {
Expand All @@ -119,8 +118,7 @@ impl UserAccount {
key,
object_ids,
gas_object_ids,
sent_certificates: Vec::new(),
received_certificates: Vec::new(),
certificates: BTreeMap::new(),
}
}
}
Expand Down Expand Up @@ -225,23 +223,18 @@ impl AccountsConfig {
.get_mut(&state.address())
.expect("Updated account should already exist");
account.object_ids = state.object_ids().clone();
account.sent_certificates = state.sent_certificates().clone();
account.received_certificates = state.received_certificates().cloned().collect();
account.certificates = state.all_certificates().clone();
}

pub fn update_for_received_transfer(&mut self, certificate: CertifiedOrder) {
match &certificate.order.kind {
OrderKind::Transfer(transfer) => {
if let Address::FastPay(recipient) = &transfer.recipient {
if let Some(config) = self.accounts.get_mut(recipient) {
if let Err(position) = config
.received_certificates
.binary_search_by_key(&certificate.order.digest(), |cert| {
cert.order.digest()
})
{
config.received_certificates.insert(position, certificate)
}
config
.certificates
.entry(certificate.order.digest())
.or_insert(certificate);
}
}
}
Expand Down
28 changes: 13 additions & 15 deletions fastpay_core/src/authority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,31 +292,29 @@ impl AuthorityState {
&self,
request: ObjectInfoRequest,
) -> Result<ObjectInfoResponse, FastPayError> {
if let Some(seq) = request.request_sequence_number {
let requested_certificate = if let Some(seq) = request.request_sequence_number {
// TODO(https://github.com/MystenLabs/fastnft/issues/123): Here we need to develop a strategy
// to provide back to the client the object digest for specific objects requested. Probably,
// we have to return the full ObjectRef and why not the actual full object here.
let obj = self
.object_state(&request.object_id)
.await
.map_err(|_| FastPayError::ObjectNotFound)?;

// Get the Transaction Digest that created the object
let transaction_digest = self
.parent(&(request.object_id, seq.increment()?, obj.digest()))
.await
let parent_iterator = self
.get_parent_iterator(request.object_id, Some(seq.increment()?))
.await?;
let (_, transaction_digest) = parent_iterator
.first()
.ok_or(FastPayError::CertificateNotfound)?;
// Get the cert from the transaction digest
let requested_certificate = Some(
self.read_certificate(&transaction_digest)
Some(
self.read_certificate(transaction_digest)
.await?
.ok_or(FastPayError::CertificateNotfound)?,
);
self.make_object_info(request.object_id, requested_certificate)
.await
)
} else {
self.make_object_info(request.object_id, None).await
}
None
};
self.make_object_info(request.object_id, requested_certificate)
.await
}
}

Expand Down
Loading

1 comment on commit 5a946cd

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

Bench results

�[0m�[0m�[1m�[32m Finished�[0m release [optimized + debuginfo] target(s) in 1.05s
�[0m�[0m�[1m�[32m Running�[0m target/release/bench
[2022-01-18T18:36:26Z INFO bench] Starting benchmark: OrdersAndCerts
[2022-01-18T18:36:26Z INFO bench] Preparing accounts.
[2022-01-18T18:36:26Z INFO bench] Open database on path: "/tmp/DB_87874BD5311F5C84EBB05748CAE5010C"
[2022-01-18T18:36:31Z INFO bench] Preparing transactions.
[2022-01-18T18:36:39Z INFO fastpay::network] Listening to Tcp traffic on 127.0.0.1:9555
[2022-01-18T18:36:40Z INFO bench] Number of TCP connections: 2
[2022-01-18T18:36:40Z INFO bench] Set max_in_flight to 500
[2022-01-18T18:36:40Z INFO bench] Sending requests.
[2022-01-18T18:36:40Z INFO fastpay::network] Sending Tcp requests to 127.0.0.1:9555
[2022-01-18T18:36:41Z INFO fastpay::network] 127.0.0.1:9555 has processed 5000 packets
[2022-01-18T18:36:42Z INFO fastpay::network] In flight 500 Remaining 35000
[2022-01-18T18:36:42Z INFO fastpay::network] 127.0.0.1:9555 has processed 10000 packets
[2022-01-18T18:36:43Z INFO fastpay::network] 127.0.0.1:9555 has processed 15000 packets
[2022-01-18T18:36:43Z INFO fastpay::network] In flight 500 Remaining 30000
[2022-01-18T18:36:44Z INFO fastpay::network] 127.0.0.1:9555 has processed 20000 packets
[2022-01-18T18:36:44Z INFO fastpay::network] 127.0.0.1:9555 has processed 25000 packets
[2022-01-18T18:36:45Z INFO fastpay::network] In flight 500 Remaining 25000
[2022-01-18T18:36:45Z INFO fastpay::network] 127.0.0.1:9555 has processed 30000 packets
[2022-01-18T18:36:46Z INFO fastpay::network] 127.0.0.1:9555 has processed 35000 packets
[2022-01-18T18:36:47Z INFO fastpay::network] In flight 500 Remaining 20000
[2022-01-18T18:36:47Z INFO fastpay::network] 127.0.0.1:9555 has processed 40000 packets
[2022-01-18T18:36:48Z INFO fastpay::network] 127.0.0.1:9555 has processed 45000 packets
[2022-01-18T18:36:48Z INFO fastpay::network] In flight 500 Remaining 15000
[2022-01-18T18:36:48Z INFO fastpay::network] 127.0.0.1:9555 has processed 50000 packets
[2022-01-18T18:36:49Z INFO fastpay::network] 127.0.0.1:9555 has processed 55000 packets
[2022-01-18T18:36:50Z INFO fastpay::network] In flight 500 Remaining 10000
[2022-01-18T18:36:50Z INFO fastpay::network] 127.0.0.1:9555 has processed 60000 packets
[2022-01-18T18:36:51Z INFO fastpay::network] 127.0.0.1:9555 has processed 65000 packets
[2022-01-18T18:36:52Z INFO fastpay::network] In flight 500 Remaining 5000
[2022-01-18T18:36:52Z INFO fastpay::network] 127.0.0.1:9555 has processed 70000 packets
[2022-01-18T18:36:52Z INFO fastpay::network] 127.0.0.1:9555 has processed 75000 packets
[2022-01-18T18:36:53Z INFO fastpay::network] Done sending Tcp requests to 127.0.0.1:9555
[2022-01-18T18:36:53Z INFO fastpay::network] 127.0.0.1:9555 has processed 80000 packets
[2022-01-18T18:36:53Z INFO bench] Received 80000 responses.
[2022-01-18T18:36:53Z WARN bench] Completed benchmark for OrdersAndCerts
Total time: 12955615us, items: 40000, tx/sec: 3087.464392852057

Please sign in to comment.