Add sv2-serde-json #1323
Add sv2-serde-json #1323
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1323 +/- ##
==========================================
+ Coverage 16.90% 18.84% +1.94%
==========================================
Files 164 169 +5
Lines 10956 11207 +251
==========================================
+ Hits 1852 2112 +260
+ Misses 9104 9095 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Bencher Report
|
| Branch | 2024-12-27-sv2-serde-json |
| Testbed | sv2 |
🚨 1 Alert
| Benchmark | Measure Units | View | Benchmark Result (Result Δ%) | Upper Boundary (Limit %) |
|---|---|---|---|---|
| client_sv2_handle_message_mining | L2 Accesses accesses | 📈 plot 🚨 alert (🔔) 🚷 threshold | 43.00 (+20.57%) | 41.90 (102.61%) |
Click to view all benchmark results
| Benchmark | Estimated Cycles | Benchmark Result 1e3 x estimated cycles (Result Δ%) | Upper Boundary 1e3 x estimated cycles (Limit %) | Instructions | Benchmark Result instructions (Result Δ%) | Upper Boundary instructions (Limit %) | L1 Accesses | Benchmark Result accesses (Result Δ%) | Upper Boundary accesses (Limit %) | L2 Accesses | Benchmark Result accesses (Result Δ%) | Upper Boundary accesses (Limit %) | RAM Accesses | Benchmark Result accesses (Result Δ%) | Upper Boundary accesses (Limit %) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| client_sv2_handle_message_common | 📈 view plot 🚷 view threshold | 2.21 (+4.20%) | 2.24 (98.35%) | 📈 view plot 🚷 view threshold | 473.00 (-0.14%) | 490.49 (96.43%) | 📈 view plot 🚷 view threshold | 730.00 (-0.81%) | 758.93 (96.19%) | 📈 view plot 🚷 view threshold | 8.00 (+55.15%) | 11.67 (68.57%) | 📈 view plot 🚷 view threshold | 41.00 (+5.96%) | 41.84 (97.99%) |
| client_sv2_handle_message_mining | 📈 view plot 🚷 view threshold | 8.33 (+1.29%) | 8.40 (99.17%) | 📈 view plot 🚷 view threshold | 2,137.00 | 📈 view plot 🚷 view threshold | 3,149.00 (-0.30%) | 3,167.64 (99.41%) | 📈 view plot 🚨 view alert (🔔) 🚷 view threshold | 43.00 (+20.57%) | 41.90 (102.61%) | 📈 view plot 🚷 view threshold | 142.00 (+1.62%) | 144.63 (98.18%) | |
| client_sv2_mining_message_submit_standard | 📈 view plot 🚷 view threshold | 6.40 (+1.58%) | 6.45 (99.23%) | 📈 view plot 🚷 view threshold | 1,750.00 (-0.04%) | 1,767.49 (99.01%) | 📈 view plot 🚷 view threshold | 2,543.00 (-0.35%) | 2,575.40 (98.74%) | 📈 view plot 🚷 view threshold | 23.00 (+33.70%) | 24.24 (94.88%) | 📈 view plot 🚷 view threshold | 107.00 (+2.16%) | 108.86 (98.29%) |
| client_sv2_mining_message_submit_standard_serialize | 📈 view plot 🚷 view threshold | 14.86 (+0.91%) | 14.94 (99.47%) | 📈 view plot 🚷 view threshold | 4,694.00 (-0.01%) | 4,711.49 (99.63%) | 📈 view plot 🚷 view threshold | 6,742.00 (-0.20%) | 6,786.82 (99.34%) | 📈 view plot 🚷 view threshold | 56.00 (+21.86%) | 63.59 (88.06%) | 📈 view plot 🚷 view threshold | 224.00 (+1.27%) | 226.20 (99.03%) |
| client_sv2_mining_message_submit_standard_serialize_deserialize | 📈 view plot 🚷 view threshold | 27.82 (+0.78%) | 28.07 (99.12%) | 📈 view plot 🚷 view threshold | 10,645.00 (+0.35%) | 10,700.14 (99.48%) | 📈 view plot 🚷 view threshold | 15,502.00 (+0.39%) | 15,596.62 (99.39%) | 📈 view plot 🚷 view threshold | 91.00 (+8.43%) | 99.35 (91.59%) | 📈 view plot 🚷 view threshold | 339.00 (+1.01%) | 343.30 (98.75%) |
| client_sv2_open_channel | 📈 view plot 🚷 view threshold | 4.49 (+1.90%) | 4.59 (97.83%) | 📈 view plot 🚷 view threshold | 1,461.00 (-0.05%) | 1,478.49 (98.82%) | 📈 view plot 🚷 view threshold | 2,154.00 (-0.31%) | 2,184.68 (98.60%) | 📈 view plot 🚷 view threshold | 12.00 (+45.45%) | 14.09 (85.18%) | 📈 view plot 🚷 view threshold | 65.00 (+3.25%) | 67.92 (95.69%) |
| client_sv2_open_channel_serialize | 📈 view plot 🚷 view threshold | 14.11 (+0.65%) | 14.20 (99.35%) | 📈 view plot 🚷 view threshold | 5,064.00 (-0.01%) | 5,081.49 (99.66%) | 📈 view plot 🚷 view threshold | 7,317.00 (-0.12%) | 7,352.77 (99.51%) | 📈 view plot 🚷 view threshold | 43.00 (+16.88%) | 48.49 (88.68%) | 📈 view plot 🚷 view threshold | 188.00 (+1.06%) | 190.91 (98.48%) |
| client_sv2_open_channel_serialize_deserialize | 📈 view plot 🚷 view threshold | 22.86 (+0.64%) | 23.08 (99.05%) | 📈 view plot 🚷 view threshold | 8,040.00 (+0.09%) | 8,057.61 (99.78%) | 📈 view plot 🚷 view threshold | 11,688.00 (+0.03%) | 11,713.51 (99.78%) | 📈 view plot 🚷 view threshold | 85.00 (+11.92%) | 90.08 (94.36%) | 📈 view plot 🚷 view threshold | 307.00 (+0.92%) | 312.93 (98.11%) |
| client_sv2_setup_connection | 📈 view plot 🚷 view threshold | 4.75 (+1.28%) | 4.79 (99.13%) | 📈 view plot 🚷 view threshold | 1,502.00 (-0.04%) | 1,519.49 (98.85%) | 📈 view plot 🚷 view threshold | 2,273.00 (-0.25%) | 2,300.87 (98.79%) | 📈 view plot 🚷 view threshold | 13.00 (+37.29%) | 15.35 (84.72%) | 📈 view plot 🚷 view threshold | 69.00 (+2.03%) | 70.21 (98.27%) |
| client_sv2_setup_connection_serialize | 📈 view plot 🚷 view threshold | 16.29 (+0.81%) | 16.33 (99.73%) | 📈 view plot 🚷 view threshold | 5,963.00 (-0.01%) | 5,980.49 (99.71%) | 📈 view plot 🚷 view threshold | 8,650.00 (-0.16%) | 8,692.77 (99.51%) | 📈 view plot 🚷 view threshold | 51.00 (+24.82%) | 56.60 (90.10%) | 📈 view plot 🚷 view threshold | 211.00 (+1.30%) | 212.29 (99.39%) |
| client_sv2_setup_connection_serialize_deserialize | 📈 view plot 🚷 view threshold | 35.78 (+0.49%) | 35.96 (99.50%) | 📈 view plot 🚷 view threshold | 14,888.00 (+0.13%) | 14,920.01 (99.79%) | 📈 view plot 🚷 view threshold | 21,867.00 (+0.10%) | 21,919.53 (99.76%) | 📈 view plot 🚷 view threshold | 109.00 (+14.95%) | 120.30 (90.60%) | 📈 view plot 🚷 view threshold | 382.00 (+0.62%) | 385.80 (99.01%) |
|
| Branch | 2024-12-27-sv2-serde-json |
| Testbed | sv1 |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result nanoseconds (ns) (Result Δ%) | Upper Boundary nanoseconds (ns) (Limit %) |
|---|---|---|---|
| client-submit-serialize | 📈 view plot 🚷 view threshold | 6,486.40 (-1.06%) | 6,991.51 (92.78%) |
| client-submit-serialize-deserialize | 📈 view plot 🚷 view threshold | 7,294.10 (-1.51%) | 7,879.34 (92.57%) |
| client-submit-serialize-deserialize-handle/client-submit-serialize-deserialize-handle | 📈 view plot 🚷 view threshold | 8,001.60 (-1.15%) | 9,550.44 (83.78%) |
| client-sv1-authorize-serialize-deserialize-handle/client-sv1-authorize-serialize-deserialize-handle | 📈 view plot 🚷 view threshold | 863.89 (-0.61%) | 951.23 (90.82%) |
| client-sv1-authorize-serialize-deserialize/client-sv1-authorize-serialize-deserialize | 📈 view plot 🚷 view threshold | 664.93 (-1.90%) | 729.63 (91.13%) |
| client-sv1-authorize-serialize/client-sv1-authorize-serialize | 📈 view plot 🚷 view threshold | 249.91 (-0.60%) | 281.60 (88.75%) |
| client-sv1-get-authorize/client-sv1-get-authorize | 📈 view plot 🚷 view threshold | 157.21 (-0.61%) | 170.07 (92.44%) |
| client-sv1-get-submit | 📈 view plot 🚷 view threshold | 6,458.50 (+1.78%) | 6,776.85 (95.30%) |
| client-sv1-get-subscribe/client-sv1-get-subscribe | 📈 view plot 🚷 view threshold | 283.76 (-1.71%) | 381.74 (74.33%) |
| client-sv1-subscribe-serialize-deserialize-handle/client-sv1-subscribe-serialize-deserialize-handle | 📈 view plot 🚷 view threshold | 731.90 (+0.51%) | 781.51 (93.65%) |
| client-sv1-subscribe-serialize-deserialize/client-sv1-subscribe-serialize-deserialize | 📈 view plot 🚷 view threshold | 609.17 (+2.59%) | 645.66 (94.35%) |
| client-sv1-subscribe-serialize/client-sv1-subscribe-serialize | 📈 view plot 🚷 view threshold | 204.41 (-1.22%) | 229.15 (89.20%) |
|
| Branch | 2024-12-27-sv2-serde-json |
| Testbed | sv1 |
Click to view all benchmark results
| Benchmark | Estimated Cycles | Benchmark Result 1e3 x estimated cycles (Result Δ%) | Upper Boundary 1e3 x estimated cycles (Limit %) | Instructions | Benchmark Result 1e3 x instructions (Result Δ%) | Upper Boundary 1e3 x instructions (Limit %) | L1 Accesses | Benchmark Result 1e3 x accesses (Result Δ%) | Upper Boundary 1e3 x accesses (Limit %) | L2 Accesses | Benchmark Result accesses (Result Δ%) | Upper Boundary accesses (Limit %) | RAM Accesses | Benchmark Result accesses (Result Δ%) | Upper Boundary accesses (Limit %) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| get_authorize | 📈 view plot 🚷 view threshold | 8.43 (-0.30%) | 8.67 (97.20%) | 📈 view plot 🚷 view threshold | 3.69 (-0.92%) | 3.86 (95.54%) | 📈 view plot 🚷 view threshold | 5.15 (-1.10%) | 5.45 (94.63%) | 📈 view plot 🚷 view threshold | 11.00 (+28.70%) | 16.47 (66.77%) | 📈 view plot 🚷 view threshold | 92.00 (+0.61%) | 96.81 (95.03%) |
| get_submit | 📈 view plot 🚷 view threshold | 95.22 (-0.14%) | 95.61 (99.59%) | 📈 view plot 🚷 view threshold | 59.35 (-0.09%) | 59.71 (99.39%) | 📈 view plot 🚷 view threshold | 85.22 (-0.10%) | 85.82 (99.30%) | 📈 view plot 🚷 view threshold | 47.00 (+4.64%) | 60.46 (77.73%) | 📈 view plot 🚷 view threshold | 279.00 (-0.63%) | 291.87 (95.59%) |
| get_subscribe | 📈 view plot 🚷 view threshold | 7.91 (-0.98%) | 8.23 (96.17%) | 📈 view plot 🚷 view threshold | 2.77 (-1.63%) | 2.94 (93.98%) | 📈 view plot 🚷 view threshold | 3.84 (-1.95%) | 4.14 (92.81%) | 📈 view plot 🚷 view threshold | 16.00 (+26.73%) | 20.95 (76.39%) | 📈 view plot 🚷 view threshold | 114.00 (-0.46%) | 117.92 (96.67%) |
| serialize_authorize | 📈 view plot 🚷 view threshold | 12.15 (-0.84%) | 12.51 (97.19%) | 📈 view plot 🚷 view threshold | 5.27 (-0.58%) | 5.43 (97.05%) | 📈 view plot 🚷 view threshold | 7.32 (-0.71%) | 7.60 (96.35%) | 📈 view plot 🚷 view threshold | 14.00 (+35.55%) | 19.02 (73.62%) | 📈 view plot 🚷 view threshold | 136.00 (-1.42%) | 143.33 (94.89%) |
| serialize_deserialize_authorize | 📈 view plot 🚷 view threshold | 24.67 (-0.18%) | 25.19 (97.95%) | 📈 view plot 🚷 view threshold | 9.84 (-0.18%) | 10.01 (98.28%) | 📈 view plot 🚷 view threshold | 13.88 (-0.23%) | 14.17 (97.89%) | 📈 view plot 🚷 view threshold | 38.00 (+5.60%) | 45.99 (82.62%) | 📈 view plot 🚷 view threshold | 303.00 (-0.21%) | 313.66 (96.60%) |
| serialize_deserialize_handle_authorize | 📈 view plot 🚷 view threshold | 30.14 (-0.58%) | 30.73 (98.10%) | 📈 view plot 🚷 view threshold | 12.02 (-0.28%) | 12.19 (98.60%) | 📈 view plot 🚷 view threshold | 17.00 (-0.34%) | 17.30 (98.29%) | 📈 view plot 🚷 view threshold | 60.00 (+7.37%) | 67.53 (88.85%) | 📈 view plot 🚷 view threshold | 367.00 (-1.07%) | 379.52 (96.70%) |
| serialize_deserialize_handle_submit | 📈 view plot 🚷 view threshold | 126.28 (-0.14%) | 126.79 (99.60%) | 📈 view plot 🚷 view threshold | 73.20 (-0.06%) | 73.53 (99.55%) | 📈 view plot 🚷 view threshold | 104.91 (-0.08%) | 105.51 (99.43%) | 📈 view plot 🚷 view threshold | 116.00 (+8.51%) | 126.06 (92.02%) | 📈 view plot 🚷 view threshold | 594.00 (-0.63%) | 610.68 (97.27%) |
| serialize_deserialize_handle_subscribe | 📈 view plot 🚷 view threshold | 27.77 (-0.48%) | 28.38 (97.85%) | 📈 view plot 🚷 view threshold | 9.58 (-0.47%) | 9.76 (98.21%) | 📈 view plot 🚷 view threshold | 13.53 (-0.57%) | 13.84 (97.81%) | 📈 view plot 🚷 view threshold | 69.00 (+6.74%) | 78.13 (88.31%) | 📈 view plot 🚷 view threshold | 397.00 (-0.56%) | 409.79 (96.88%) |
| serialize_deserialize_submit | 📈 view plot 🚷 view threshold | 115.14 (-0.09%) | 115.71 (99.51%) | 📈 view plot 🚷 view threshold | 68.06 (+0.01%) | 68.42 (99.47%) | 📈 view plot 🚷 view threshold | 97.65 (+0.00%) | 98.29 (99.35%) | 📈 view plot 🚷 view threshold | 69.00 (+4.93%) | 87.84 (78.56%) | 📈 view plot 🚷 view threshold | 490.00 (-0.70%) | 505.36 (96.96%) |
| serialize_deserialize_subscribe | 📈 view plot 🚷 view threshold | 23.21 (-0.46%) | 23.82 (97.47%) | 📈 view plot 🚷 view threshold | 8.14 (-0.50%) | 8.32 (97.93%) | 📈 view plot 🚷 view threshold | 11.45 (-0.60%) | 11.75 (97.47%) | 📈 view plot 🚷 view threshold | 43.00 (+9.90%) | 51.32 (83.79%) | 📈 view plot 🚷 view threshold | 330.00 (-0.50%) | 342.45 (96.36%) |
| serialize_submit | 📈 view plot 🚷 view threshold | 99.61 (-0.18%) | 100.09 (99.52%) | 📈 view plot 🚷 view threshold | 61.41 (-0.07%) | 61.73 (99.47%) | 📈 view plot 🚷 view threshold | 88.08 (-0.08%) | 88.65 (99.36%) | 📈 view plot 🚷 view threshold | 51.00 (+5.56%) | 67.16 (75.93%) | 📈 view plot 🚷 view threshold | 322.00 (-1.05%) | 337.63 (95.37%) |
| serialize_subscribe | 📈 view plot 🚷 view threshold | 11.36 (-0.38%) | 11.60 (97.91%) | 📈 view plot 🚷 view threshold | 4.12 (-1.01%) | 4.28 (96.16%) | 📈 view plot 🚷 view threshold | 5.71 (-1.27%) | 6.00 (95.14%) | 📈 view plot 🚷 view threshold | 17.00 (+20.96%) | 24.45 (69.53%) | 📈 view plot 🚷 view threshold | 159.00 (+0.28%) | 164.64 (96.57%) |
|
What crates will depend on this new |
This change would only impact |
For reference only, I did a Stratum v1 no_std client implementation (https://github.com/Foundation-Devices/foundation-rs/tree/main/stratum-v1) and the json parsing was a nightmare... |
Also I was confused because this new crate have the "sv2" prefix, if it is sv1 oriented (JSON data) or at least not specific to sv2, maybe we should use this prefix? |
makes sense. Its still in POC phase, but if we end up taking this approach I gonna change this name, |
As you may know, we have two versions of
The coexistence of these two implementations is challenging due to conflicting APIs for byte serialization and deserialization. In one of our previous meeting, we decided to develop our own custom JSON serializer and deserializer to replace |
I am still kinda confused. Why actually the message generator need ser/de if its working with sv2 and we already know how to handle sv2 message? Also, in case MG need to do this ser/de, why not add |
MG need serde cause tests are defined as json so it parse one or more json files and then execute them. In order to parse these files it need to do JSON <-> Sv2. Another example of where having JSON <-> Sv2 is helpful is here: https://github.com/demand-open-source/demand-test-data-collector (is outdated and just a POC that I did in order to try the demand-easy-sv2 library, but I can see it's utility) where all the messages intercepted by a proxy are saved as json. |
|
I see. Why then not add serde to MG and thats it? Why adding serde to MG trickledown to many different crates in the past? |
I don't get the question. If you are asking why we added serde to the binary-sv2 crates in first place, the reason is that,at the start, I did it with serde, then I have been asked to remove serde from it^1, and in order to do it I wrote a completely different library that do not use serde but that expose the same API of the serde one (that cause it was already used in other places). Next step would have been to completely remove serde and we started it, but then it turned out that the library with serde was very handy for the MG so we decided to keep it. Keep in mind that at the time we had only one big workspace and MG was part of it. ^ 1 this was necessary cause the original plan were to use the SRI libs in bitcoind in order to implement sv2, then we changed our minds. |
|
@Shourya742 after reading again the discussion here, I think the best approach is the one you started: developing a separate specific crate that can be then imported by the Are you facing some particular issues somewhere? Is there anything else you would like to get feedback about? |
Shourya742
force-pushed
the
2024-12-27-sv2-serde-json
branch
from
9388cf8 to
ae2807d
Compare
Shourya742
force-pushed
the
2024-12-27-sv2-serde-json
branch
from
191b21b to
89e0c58
Compare
Shourya742
force-pushed
the
2024-12-27-sv2-serde-json
branch
from
e14205f to
ede3466
Compare
| @@ -0,0 +1,7 @@ | |||
| [package] | |||
| name = "sv2_serde_json" | |||
There was a problem hiding this comment.
should we use a more generic name here? this implies it's sv2 only (not sv1)
|
@Shourya742 should we close this for now and reopen in MG repo? |
Yes, @plebhash and I discussed this PR. We can close it, but I was waiting for today's dev sync call to finalize the path forward for MG. |
This PR introduces a sv2-serde-json as a utility module. The goal is to eliminate the dependency on
serdefrom the message generator, ultimately phasing outbinary_sv2_serde. Currently,sv2_serde_jsonhandles serialization and deserialization by converting data into aValueconstruct, which must then be mapped onto Rust's structs and enums.By adopting this approach, we aim to remove all dependencies on
serde, paving the way for the eventual deprecation ofbinary_sv2_serdeentirely. Alternatively, if we decide not to proceed with this new utility, we will extend the existingbinary_sv2to support JSON serialization and deserialization directly.This PR is marked as a draft while we finalize the chosen approach. In the meantime, this branch will be maintained for developing JSON features using the external utility, rather than extending
binary_sv2.