Add transaction handler RPC for Mysticeti fastpath #19401
Conversation
This will not function until consensus adapter/handler side is implemented.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
| &self, | ||
| request: tonic::Request<HandleTransactionRequestV2>, | ||
| // TODO: Do we want to make a new response type for this? | ||
| ) -> WrappedServiceResponse<HandleCertificateResponseV3> { |
There was a problem hiding this comment.
It seems more refactoring will be needed to support soft bundle. Most of the logic here should be shared between handle_transaction_v2 and handle_soft_bundle_transactions_v2 handlers. Or we can take the opportunity to support batched transaction submission with a single handler, which should cover the soft bundle use case as well.
There was a problem hiding this comment.
Yes, I was not originally planning to support soft bundle for the first prototype. do you think that's worth adding to this PR?
There was a problem hiding this comment.
As discussed, we can definitely can back and refactor later.
| } | ||
|
|
||
| #[instrument(level = "trace", skip_all)] | ||
| pub async fn handle_transaction_v2( |
There was a problem hiding this comment.
This doesn't seem to be the right naming anymore since we are only doing various checks here.
There was a problem hiding this comment.
Do you have an alternate suggestion for name?
I think actually the existing handle_transaction was poorly named and should have been called sign_transaction. But for the new one I'm adding, handle is more apt because we really are handling the full transaction e2e: sending an unsigned tx through consensus, executing it, and returning the effects. I didn't think temporary confusion of naming overload would be too bad, bc eventually we plan to remove the old version.
I don't think anything changes here? We aren't acquiring locks for the tx in the RPC handler anymore, that will need to be done on the consensus handler side |
4 participants
This will not function until consensus adapter/handler side is implemented.