From 81479b6be9af86d2c69b53295724bd44ee3b5f91 Mon Sep 17 00:00:00 2001 From: bhargavbh <2bhargav5@gmail.com> Date: Wed, 16 Nov 2022 11:25:28 +0100 Subject: [PATCH 1/2] tuned --- .../c++polkadot-light-client_1_bhargavbh.md | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/evaluations/c++polkadot-light-client_1_bhargavbh.md b/evaluations/c++polkadot-light-client_1_bhargavbh.md index f55ce73383..25d6f28b22 100644 --- a/evaluations/c++polkadot-light-client_1_bhargavbh.md +++ b/evaluations/c++polkadot-light-client_1_bhargavbh.md @@ -1,30 +1,19 @@ # Evaluation -> Don't remove any of the mandatory parts presented in bold letters or as headlines! -> When evaluating the last milestone of a grant, check if the finished grant refers to an [RFP](https://github.com/w3f/Grants-Program/blob/master/rfps/README.md) and move the corresponding RFP to [implemented](https://github.com/w3f/Grants-Program/tree/master/rfps/implemented). -> Lines starting with `>`, such as this one, can be removed. - - **Status:** In Progress - **Application Document:** https://github.com/w3f/Grants-Program/blob/master/applications/c%2B%2Bpolkadot-light-client.md - **Milestone:** 1 | Number | Deliverable | Accepted | Link | Evaluation Notes | | ------ | ----------- | -------- | ---- |----------------- | -| 1. | Cryptography ||...| Missing a clear comparison of required components between Light-Client and Full Node. Ideally describe a spectrum of functionalities that a light-client can support. -| 2. | Runtime Environment Requirements ||...| No discussion of embedding runtime environments to run on-chain wasm blobs. How would the compiled targets (for different platforms) interface with the embedded runtime environments. -| 3. | Multichain Support ||...| No mention of chains to be supported (e.g. Polkadot, Kusama, Parachains) -| 4. | JSON-RPC APIs ||...| Mentions three initial RPC methods to be implemented. Would this suffice? Justification would be appreciated. -| 5. | Dependencies ||...| Details of the required crypto libs missing; nice to have: assessment of using the heavy crypto libs in a resource constrained light-client. +| 1. | Cryptography ||...| comparisons and security advantages delivered at a high-level +| 2. | Runtime Environment Requirements ||...| Propose to use binaryen for compiling/interpreting wasm runtime calls. . +| 3. | Multichain Support ||...| Mentions testing of compatibility with Westend, Kusama, and Polkadot. No mention of XCM use cases. +| 4. | JSON-RPC APIs ||...| (wait for florian's feedback) +| 5. | Dependencies ||...| Technically speaking, the delivery passes this. Quite sparse for crypto, relatively a bit more detailed for networking. ## General Remarks -Overall, the spec seems to be sparse and lacks details on several key components of the light-client. Components like Runtime/execution environments, local host storage, light-weight interactions with GRANDPA and BABE (and Sassafras in future) for the light client have not been addressed. -It would also be interesting to gain insights into the disadvantages of implementing in C++ and the major pain points the team expects. Furthermore, the document could be improved by adding justification to the design choices. For example, -> *"We will use hash_map with serialization/deserialization to hold the blockchain state."* in Implementation.md - -Missing details, especially on the limits of using a hashmap. What were the other options considered? would there not be any non-persistence issues? AFAIK, other node implementations use a Database for similar tasks. +The concerns raised in previous evaluation have been by and large addressed. However, the delivery is more of an implementation roadmap than a Light Client Spec that can be integrated into [Polkadot Spec](https://spec.polkadot.network). This evaluation has also taken into consideration the initial strides in implementation taken towards a Light Client [here](https://github.com/eqlabs/polkadot-light-client-spec/). We suggest integrating the implementation repo into your delivery and making necessary amendments (e.g removing claims of integration into Polkadot Spec, mention of XCM usecases) to the initial Grant proposal via a PR. -Similarly, it is mentioned: -> *“Boost::asio used as the main event loop”* - Diagram -Why does ASIO satisfy the requirements at hand? What would the Callback Graph of the event-driven system look like? What are the event triggers? From 83f36c00eac38ab1deb69ec0c31bdf14c7985c0d Mon Sep 17 00:00:00 2001 From: bhargavbh <2bhargav5@gmail.com> Date: Wed, 16 Nov 2022 13:49:53 +0100 Subject: [PATCH 2/2] tuned --- evaluations/c++polkadot-light-client_1_bhargavbh.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/evaluations/c++polkadot-light-client_1_bhargavbh.md b/evaluations/c++polkadot-light-client_1_bhargavbh.md index 25d6f28b22..8981b47197 100644 --- a/evaluations/c++polkadot-light-client_1_bhargavbh.md +++ b/evaluations/c++polkadot-light-client_1_bhargavbh.md @@ -8,12 +8,11 @@ | ------ | ----------- | -------- | ---- |----------------- | | 1. | Cryptography ||...| comparisons and security advantages delivered at a high-level | 2. | Runtime Environment Requirements ||...| Propose to use binaryen for compiling/interpreting wasm runtime calls. . -| 3. | Multichain Support ||...| Mentions testing of compatibility with Westend, Kusama, and Polkadot. No mention of XCM use cases. -| 4. | JSON-RPC APIs ||...| (wait for florian's feedback) -| 5. | Dependencies ||...| Technically speaking, the delivery passes this. Quite sparse for crypto, relatively a bit more detailed for networking. +| 3. | Multichain Support ||...| Mentions testing of compatibility with Westend, Kusama, and Polkadot. No mention of XCM use cases. +| 4. | JSON-RPC APIs ||...| Transactions related methods missing. Cleanest way to integrate with the browser is implementing the SC extension protocol [here](https://github.com/paritytech/substrate-connect/tree/main/packages/connect-extension-protocol). We are aware that the JRPC interface is unstable, but for completeness would make sense to add the other RPC methods to the minimal set. Also, please details in stages of implementation. +| 5. | Dependencies ||...| Technically speaking, the delivery passes. Quite sparse for crypto, relatively a bit more detailed for networking. ## General Remarks -The concerns raised in previous evaluation have been by and large addressed. However, the delivery is more of an implementation roadmap than a Light Client Spec that can be integrated into [Polkadot Spec](https://spec.polkadot.network). This evaluation has also taken into consideration the initial strides in implementation taken towards a Light Client [here](https://github.com/eqlabs/polkadot-light-client-spec/). We suggest integrating the implementation repo into your delivery and making necessary amendments (e.g removing claims of integration into Polkadot Spec, mention of XCM usecases) to the initial Grant proposal via a PR. - +The concerns raised in previous evaluation have been by and large addressed. However, the delivery is more of an implementation roadmap than a Light Client Spec that can be integrated into [Polkadot Spec](https://spec.polkadot.network/). We have a few minor suggestions mentioned above, please include them. We have also taken into consideration the initial strides towards implementing a Light Client [here](https://github.com/eqlabs/polkadot-light-client-spec/). We suggest integrating it into your delivery and making necessary amendments (e.g removing claims of integration into Polkadot Spec) to the initial Grant proposal via a PR.