-
Notifications
You must be signed in to change notification settings - Fork 11.2k
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
Add QuorumDriver #2515
Add QuorumDriver #2515
Conversation
pub enum ExecuteTransactionResponse { | ||
ImmediateReturn, | ||
TxCert(Box<CertifiedTransaction>), | ||
EffectsCert(Box<(CertifiedTransaction, TransactionEffects)>), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit misleadingly named, isn't it? The TransactionEffects here is unsigned, so the client can't use this as a proof of finality. An effects cert would actually be a TransactionEffectsEnvelope<AuthorityQuorumSignInfo>
, right?
FWIW I may look at forming effects certificates in full node, since I'm already going to be doing the work of waiting for finality before syncing a cert. I say "may" because fullnode can get away with merely proving finality to itself by observed 2f+1 idential effects digests - forming an fx cert involves further fetching 2f+1 signed effects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The plan here is to make it return TransactionEffectsEnvelope eventually.
Currently since authority_aggregator only returns effects, here I am leaving it as it is. I plan to send future PRs to actually make authority_aggregator return effects certs.
crates/sui-quorum-driver/src/lib.rs
Outdated
.instrument(tracing::debug_span!("process_cert")) | ||
.await?; | ||
Ok(ExecuteTransactionResponse::EffectsCert(Box::new(response))) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Evan built a very similar pattern for sending commands over a channel for lock_service.rs, I wonder if its worth abstracting that mechanism so we don't keep re-inventing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
objects.json, owned_objects.json and transactions.json doesn't have real changes, all changes are due to random ids and transaction hash, we can revert the changes.
Address all feedback:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This PR adds a new component called QuorumDriver.
It provides 3 interface functions: