Grant-Milestone-Delivery
Grant-Milestone-Delivery copied to clipboard
add: substrate_core_polywrapper-milestone_1.md
Milestone Delivery Checklist
- [x] The milestone-delivery-template.md has been copied and updated.
- [x] The invoice form :pencil: has been filled out for this milestone.
- [ ] This pull request is being made by the same account as the accepted application.
- [ ] In case of acceptance, the payment will be transferred to the BTC/ETH payment address provided in the application.
- [x] The delivery is according to the Guidelines for Milestone Deliverables.
Link to the application pull request: https://github.com/w3f/Grants-Program/pull/676
Thank you for the delivery @empea-careercriminal. Could @mpetrunic confirm that you're authorised to make the delivery, since he originally submitted the application? And if you want to receive payment to a different address, could you please submit an amendment and modify it in the application file? Ideally, @mpetrunic would also sign off on that.
Thank you for the delivery @empea-careercriminal. Could @mpetrunic confirm that you're authorised to make the delivery, since he originally submitted the application? And if you want to receive payment to a different address, could you please submit an amendment and modify it in the application file? Ideally, @mpetrunic would also sign off on that.
Confirming :)
@alxs the address for payment is the one indicated in the invoice that we uploaded. Where can I check if this matches you records or if we need to file an amendment for this?
@empea-careercriminal you provided one in your application.
The address indicated in the application is fine. Please use that.
Hey @empea-careercriminal, could you please take a moment to go through the original application and either submit an amendment such that the described functionality matches that of the current delivery, or make changes to the delivery? For example, deployment to IPFS, some functionality for substrate-signer-provider
looks like it hasn't been implemented, the API for substrate-core-wrapper
looks different, etc. For implementation changes with no difference in scope such as possibly the last one, you can instead provide a summary of changes. If there are any other differences, please include them in the amendment/delivery now as that will make the rest of the evaluation process easier for both of us.
Hi @alxs. I'm one of the engineers who worked on the integration so I'll respond on behalf of @empea-careercriminal.
-
IPFS Deployment - The wrapper is not ready for deployment to IPFS (as listed in the M1 submission). I'm also not sure if this is something that should be handled by the devs and might be better as a CD task in the polywrap repo. I will amend the original application to omit this task
-
signer-provider plugin - This component is complete and exposes all signing functionality provided by the polkadot-js browser extension. I believe the original proposal was flawed and included too much scope for this component. As it is implemented it only handles the signing interaction with the browser plugin which is a much cleaner seperation of concerns.
-
rpc-wrapper API - The list of functions givn in the proposal appears to be a list of Substrate RPC methods. This is a bit misleading and I'm not sure why it was written this way. The API we have chosen does provide all of the functionality of these RPC methods and some additional helpers. The proposal has been updated to include the actual interface instead.
I've submitted an amendment to the application to help clarify and removed the IPFS submission requirement. Please see the PR in w3f/Grants-Program#1262
Thanks and I hope this clears thing up.
@willemolding thank you for the answer and submitting the amendment. Could you please update the delivery file and/or the amendment such that the lists of deliverables match? Note that deliverables 0a-d. are mandatory and need to be present in both. Most of these seem to be missing from the delivery, for instance tutorial, testing guide and article. Please also have a look at our milestone delivery guidelines.
Besides, there seem to be a number of unresolved issues with the Substrate core wrapper. Is this information outdated?
@alxs thank you for your feedbacks. We are working on those with our engineering and the Polywrap teams. Regarding the unresolved issue, this information are outdated indeed.
@alxs We updated the codebase and here's the corresponding article. Would you take a look at both of this?
I am aware of that pending CLA agreement and waiting for this to be resolved soon.
On hold pending approval of the amendment.
Thank you both! I can't provide feedback on the article myself, but if you want us to proofread it and/or retweet an announcement about it, you can send it to [email protected].
Regarding the codebase, I'll go over the delivery again as soon as the amendment has been accepted. A reminder to also update the delivery file.
@alxs Just checking if you have got all you need from our side to evaluate the amendment? Please ping in case something is blocking.
Thanks @empea-careercriminal, see my comment in the amendment PR. The list of deliverables in the delivery file and the application need to match (you may want to update the delivery file instead). Feel free to argue against the reduction in price.
@alxs Just checking in to ask if this is good now, or if you require other changes, so this can be approved?
@empea-careercriminal thanks for following up. I would also like to test its usage, which I tried to do as shown in the example here. If I'm not mistaken, for now substrate-signer-provider-plugin-js
and @polywrap/substrate-rpc-wrapper
need to be local dependencies, and the wrapper needs to be built before being used. However, I tried following the instructions here and got the following errors with yarn
:
...
error An unexpected error occurred: "https://registry.yarnpkg.com/substrate-signer-provider-plugin-js: Not found".
info If you think this is a bug, please open a bug report with the information provided in "/Users/aleixo/repos/tmp/integrations/protocol/substrate/rpc-wrapper/yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
and yarn codegen
:
yarn run v1.22.15
$ npx polywrap codegen
🔄 Manifest loaded from ./polywrap.yaml
✅ Manifest loaded from ./polywrap.yaml
🔄 Generate types
❌ Failed to generate types: Error: ABI not found at /Users/aleixo/repos/tmp/integrations/protocol/substrate/node_modules/@polywrap/http-interface/build/wrap.info
Error: ABI not found at /Users/aleixo/repos/tmp/integrations/protocol/substrate/node_modules/@polywrap/http-interface/build/wrap.info
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Do you have a working example somewhere, or could you guide me through this?
@dOrgJelli @willemolding Would you mind providing some guidance?
Hey apologies for the wait, just seeing this now @alxs @empea-careercriminal.
So the plugin is not currently published as it's in a pre-release state ATM, so yes in order to build the wrapper you need to have the plugin locally. We explain this here: https://github.com/polywrap/integrations/tree/main/protocol/substrate#developer-setup
If you'd like to verify the wrapper & plugin both build locally, you can follow these steps:
git clone https://github.com/polywrap/integrations
cd integrations/protocol/substrate
nvm use
yarn
cd ./signer-provider-js
yarn build
cd ../rpc-wrapper
yarn build
It might be useful to see the CI steps that are run, which show this, and how to run tests successfully: https://github.com/polywrap/integrations/blob/d5cdebe13c33ae9623c4e7726c720c579b458c56/.github/workflows/substrate-ci.yaml#L27-L37
To avoid this entirely, I've published a pre-release version of the plugin to NPM here: https://www.npmjs.com/package/substrate-signer-provider-plugin-js
I've created a demo application to show the wrapper working in an end user application here: https://github.com/dOrgJelli/substrate-wrapper-test
You can run this example by simply running:
git clone https://github.com/dorgjelli/substrate-wrapper-test
cd substrate-wrapper-test
nvm use
yarn
yarn build
yarn test
Which should output:
...local node setup (docker output)...
****************************************
Block Hash: 0x1ece48cf1ebfadae2a8a40b2c2f3c580f51f2babdf1523a80aa5e1c050411cfe
****************************************
...local node tear down (docker output)...
And the code which emits this can be found here: https://github.com/dOrgJelli/substrate-wrapper-test/blob/fb9d075768d9e2fecbf97f0f3b37cda3938f5640/src/index.ts#L8-L17
Happy to answer any further questions.
Thanks a lot for the instructions and for setting up the example project. That was very helpful.
Regarding testing, it would be great to have tests for cases other than the happy path and those checking that a call doesn't return any errors. The one that was commented out is also still being skipped, which I think just needs to be fixed?
Lastly, the rpc-wrapper
tests fail to run for me with following output: yarn test.txt.
Note that @keeganquigley will be taking over this evaluation in the new year. I believe the milestone can be accepted once the above has been addressed. My evaluation notes can be found here.
Thank you @alxs for the review. With regards to the remaining issues, may I ask for your feedback @dOrgJelli?
Hi @empea-careercriminal @dOrgJelli thanks for the changes. I am able to get all tests to run including rpc-wrapper
. You can see my final notes here. That leaves the only remaining issue to be the skipped test that Alex mentioned above. Is there any way this test can be fixed? Also feel free to add more paths, as mentioned in the comment above. Thanks!
Hello @keeganquigley , I work with @empea-careercriminal and we're looking into it. I'll have more for you soon!
Hi @stonecharioteer are you able to give any updates?
Hi @keeganquigley, yes, we were working on this. Here's the PR
- Refactored the implementation of extrinsic API
- Fixed the JsonRPC API of
author_submit_extrinsic
- Use
sr25519
keypair in e2e tests
Note that this will have some breaking changes, since the extrinsic APIs in rpc-wrapper
are modified.
thanks @stonecharioteer much appreciated. I see the PR is merged, does this mean I can continue re-evaluating the milestone or are we still waiting on more changes?
Yes @keeganquigley, you should be good to go. The failing test that was halting evaluation is now functional, and running in CI successfully. You can see a run of this here: https://github.com/polywrap/integrations/actions/runs/4094209269/jobs/7060221782
Or just test locally, steps should not have changed:
git clone https://github.com/polywrap/integrations
cd integrations/protocol/substrate
nvm use
yarn
cd ./signer-provider-js
yarn build
yarn test
cd ../rpc-wrapper
yarn build
yarn test
Please let us know if you have any issues! :)
thanks for the changes @dOrgJelli much appreciated. Everything works now! I'm happy to accept this milestone, and you can find my final evaluation notes here.
Cheers and looking forward to the next one.
@empea-careercriminal @dOrgJelli @stonecharioteer apologies for not realizing this earlier, but your invoice doesn't include the VAT ID on it, which is required for payment. Could you please submit an updated invoice with the W3F VAT ID included? You can find this by clicking through the invoice form.
Thank you!
We noticed that this is the last milestone of your project, so what are the potential next steps?
While you can always apply for a follow-up grant, the grants program might not be the best tool for your future growth. Our main goal is to support research as well as early-stage technical projects. Below are a couple of other options in our ecosystem that might be better suited as the next step, depending on your goals:
Project with a Business Case/Token: Substrate Builders Program or VC Funding
Common Good Projects: Treasury Funding
Let us know if you have any questions regarding the above. We are more than happy to point you to additional resources and help you determine the best course of action.
Congratulations on completing the first milestone of this grant! As part of the Grants Program, we want to help grant recipients acknowledge their grants publicly. To that end, we’ve created a badge for projects that successfully deliver their first milestone. Please use the badge only in reference to the work that has been completed as part of this grant, so please do not display it on your team or project's homepage unless accompanied by a short description of the grant. Furthermore, you're now welcome to announce the grant publicly. Please remember to observe the foundation’s guidelines in doing so. If you haven't already, reach out to [email protected] for feedback on your announcement and cross-promotion.
Thank you for your contribution, and good luck with the remaining milestones, if any! As usual, let us know if you encounter any delays during your remaining milestones, by leaving a comment on the application PR or submitting an amendment.
Thank you for the updated invoice! It has been forwarded internally for payment.