chore(p2p_service): clean up processing of p2p req/res protocol #2124
Conversation
crates/services/p2p/src/service.rs
Outdated
| request_id: InboundRequestId, | ||
| response_sender: fn(Option<R>) -> ResponseMessage, | ||
| db_lookup: F, | ||
| task_request: fn(Option<R>, InboundRequestId) -> TaskRequest, |
There was a problem hiding this comment.
here I preferred using fn pointers for the response_sender, task_request instead of trait methods on RequestMessage to reduce verbosity of the code. Happy to implement it that way if the team prefers it.
There was a problem hiding this comment.
I'm happy with taking functions as arguments, although since there are three related functions here, grouping them together in a trait might make the code even nicer to read.
One question though, response_sender and task_request are both function pointers whereas db_lookup is a FnOnce-generic. I think the code would read better if we used generics for all three arguments. This would make the function more flexible for future extensions as well.
There was a problem hiding this comment.
addressed in e52bffc, thanks
cc: @AurelienFT
crates/services/p2p/src/service.rs
Outdated
| request_id: InboundRequestId, | ||
| response_sender: fn(Option<R>) -> ResponseMessage, | ||
| db_lookup: F, | ||
| task_request: fn(Option<R>, InboundRequestId) -> TaskRequest, |
There was a problem hiding this comment.
I'm happy with taking functions as arguments, although since there are three related functions here, grouping them together in a trait might make the code even nicer to read.
One question though, response_sender and task_request are both function pointers whereas db_lookup is a FnOnce-generic. I think the code would read better if we used generics for all three arguments. This would make the function more flexible for future extensions as well.
There was a problem hiding this comment.
I like the refactor. I agree with @netrome on using the same sematic for the 3 functions as parameter.
Little question: there is changes pushed to state_transition_bytecode.wasm I don't know well the purpose of this file, is this change required ?
Co-authored-by: Green Baneling <XgreenX9999@gmail.com>
This is generated when running |
…essing' into fix/large-match-arms-in-p2p-processing
## Version v0.35.0 ### Added - [2122](#2122): Changed the relayer URI address to be a vector and use a quorum provider. The `relayer` argument now supports multiple URLs to fetch information from different sources. - [2119](#2119): GraphQL query fields for retrieving information about upgrades. ### Changed - [2113](#2113): Modify the way the gas price service and shared algo is initialized to have some default value based on best guess instead of `None`, and initialize service before graphql. - [2112](#2112): Alter the way the sealed blocks are fetched with a given height. - [2120](#2120): Added `submitAndAwaitStatus` subscription endpoint which returns the `SubmittedStatus` after the transaction is submitted as well as the `TransactionStatus` subscription. - [2115](#2115): Add test for `SignMode` `is_available` method. - [2124](#2124): Generalize the way p2p req/res protocol handles requests. #### Breaking - [2040](#2040): Added full `no_std` support state transition related crates. The crates now require the "alloc" feature to be enabled. Following crates are affected: - `fuel-core-types` - `fuel-core-storage` - `fuel-core-executor` - [2116](#2116): Replace `H160` in config and cli options of relayer by `Bytes20` of `fuel-types` ### Fixed - [2134](#2134): Perform RecoveryID normalization for AWS KMS -generated signatures. ## What's Changed * Change TODO clippy to wait for false positive correction by @AurelienFT in #2110 * Remove option on shared algo, give default algo, reorder service startup by @MitchTurner in #2113 * chore(sealed_blocks): optimize fetching of sealed block header at a given height by @rymnc in #2112 * chore(p2p_service): remove unnecessary cast to usize by @rymnc in #2123 * Added a benchmark for the predicate with ed19 verification by @xgreenx in #2127 * Add test SignMode is_available by @AurelienFT in #2115 * feat: Add graphql query fields for retreiving information about upgrades by @netrome in #2119 * Small code optimizations by @MoneyBund in #2035 * Change relayer URI address to be vector and use quorum provider by @AurelienFT in #2122 * chore(p2p_service): clean up processing of p2p req/res protocol by @rymnc in #2124 * feat: add `submitAndAwaitStatus` subscription endpoint by @maschad in #2120 * Replace H160 in config and cli options of relayer by Bytes20 by @AurelienFT in #2116 * Secp256k1 RecoveryID correction for KMS-genrated signatures by @Dentosal in #2134 * Added `no_std` support state transition related crates by @xgreenx in #2040 ## New Contributors * @MoneyBund made their first contribution in #2035 * @maschad made their first contribution in #2120 **Full Changelog**: v0.34.0...v0.35.0
Linked Issues/PRs
Description
Cleans up the impl for
TaskRequestby breaking the processing into digestable but extendable functionsChecklist
Before requesting review
After merging, notify other teams
[Add or remove entries as needed]