[fastx client / auth] Add OrderInfoRequest to authority and use it for more robust sync.
#311
Conversation
gdanezis
changed the title
OrderInfoRequest to authority and use it for more robust sync.
| @@ -207,6 +212,9 @@ impl Storage for AuthorityTemporaryStore { | |||
| } | |||
| } | |||
|
|
|||
| // The adapter is not very disciplined at filling in the correct | |||
| // previous transaction digest, so we ensure it is correct here. | |||
| object.previous_transaction = self.tx_digest; | |||
There was a problem hiding this comment.
Good catch! Doesn't have to be here, but I think we should also move the increment of version here to ensure that all the metadata update happens in one place (and I'm happy to tackle this--should be a small change). That will get us closer to a coherent discipline.
There was a problem hiding this comment.
My only worry is that objects before they are written to the temp_store are going around with the incorrect previous_transaction. As per issue: #312
| pub events: Vec<Event>, | ||
| /// The set of transaction digests this order depends on. | ||
| pub dependencies: Vec<TransactionDigest>, |
There was a problem hiding this comment.
Wondering if we need this. If the client knows the input objects to this transaction (as we'd expect in practice), can't it derive the dependencies of an order on its own?
Further: my mental model of OrderEffects is "the things that happened when the transaction got executed" (which I think is also suggested by the name). The other fields fit into this worldview--i.e., they are not knowable without executing the transaction (with the exception of transaction_digest, which I think we could also consider eliminating). dependencies also does not fit into this view because it can be known without executing the transaction.
There was a problem hiding this comment.
Ok, this is a tough one.
We need it somewhere that is either signed or within the chain of hashes from the latest objects / transactions to the genesis. The ideal place to put it would be within the certificate. This should in theory be possible, since the client should have the objects that are input into the cert, and is able to infer dependencies. But right now we are not at this place yet on the client side.
There was a problem hiding this comment.
Got it + very much aligned on the short-term and long-term plan.
gdanezis
force-pushed
the
order-info-flow
branch
from
7d4bed3 to
54a0468
Compare
OrderRequestInfoon the authority that retrieves anOrderInfoResponsebyTransactionDigest.dependenciesvector listing the transaction digests of the transactions that preceded this one, ie that are necessary to construct the version of the objects it relies on.previous_transactionfield of mutated objects not being updated by the adaptor.