Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hyperfridge Milestone 2 #1169

Merged
merged 6 commits into from
May 7, 2024
Merged

Hyperfridge Milestone 2 #1169

merged 6 commits into from
May 7, 2024

Conversation

wasabrot
Copy link
Contributor

Milestone Delivery Checklist

  • The milestone-delivery-template.md has been copied and updated.
  • This pull request is being made by the same account as the accepted application.
  • I have disclosed any and all sources of reused code in the submitted repositories and have done my due diligence to meet its license requirements.
  • In case of acceptance, invoices must be submitted and payments will be transferred to the Polkadot AssetHub and/or fiat account provided in the application.
  • The delivery is according to the Guidelines for Milestone Deliverables.

Link to the application pull request: w3f/Grants-Program#2096

@wasabrot
Copy link
Contributor Author

Hello - the author check is still failing; not sure why. As I understood, the merged hyperfridge.md must have same authors as this PR; we tried twice with wasabrot and wstrametz and still it fails, even if they are listed as authors. Can you give us a hint where to look? Maybe @keeganquigley ?

@semuelle
Copy link
Member

Hello - the author check is still failing; not sure why. As I understood, the merged hyperfridge.md must have same authors as this PR; we tried twice with wasabrot and wstrametz and still it fails, even if they are listed as authors.

Hi @wasabrot, thanks for the delivery and sorry for the inconvenience. I'm not sure why it's failing, but I can see that you are both authors of the application, so no need to fix it. Someone will look into the delivery soon.

@PieWol PieWol self-assigned this Apr 30, 2024
@PieWol
Copy link
Member

PieWol commented Apr 30, 2024

Hey @wasabrot ,
I see in your delivery that you are planning to change your milestones. This needs to be done with an amendment to the application itself. So please put up an amendment PR. See here

@PieWol
Copy link
Member

PieWol commented Apr 30, 2024

Hey @wasabrot ,
please update the links in your delivery. Most links are not pointing to actual files right now. Please ping me as soon as you changed them so I can continue the evaluation. Thanks!

@wasabrot
Copy link
Contributor Author

wasabrot commented May 3, 2024

Hey @wasabrot , please update the links in your delivery. Most links are not pointing to actual files right now. Please ping me as soon as you changed them so I can continue the evaluation. Thanks!

Hi @PieWol - fixed broken links, sorry for the blunder.

@PieWol
Copy link
Member

PieWol commented May 3, 2024

Hey @wasabrot ,
I'm a little confused as these tests here are all 3y old. Which is the correct dir for the tests you have created for this very milestone? If these are the tests you are using they are missing inline documentation. Here is my current evaluation. Otherwise good job!

@wasabrot
Copy link
Contributor Author

wasabrot commented May 3, 2024

Hey @wasabrot , I'm a little confused as these tests here are all 3y old. Which is the correct dir for the tests you have created for this very milestone? If these are the tests you are using they are missing inline documentation. Here is my current evaluation. Otherwise good job!

Hi @PieWol - totally see your point! To give some background - we needed to integrate the ZK proof (which is tested in the risc-zero project), so no extra tests for that. And we wanted to only little change in the backend, so that we do not have to rewrite the OCW. But on top of this milestone we had to change the LibEuFin repo (the banking backend) and the proof generation a little, plus a small watchdog to trigger proof generation, which was not listed in the delivery. Bit more "flesh" will be in coming milestones, where we want to integrate the asset hub (@dastansam is working on the amendment). Thx a lot!

@PieWol
Copy link
Member

PieWol commented May 3, 2024

Ok, what about the aspect of the license? @wasabrot

Fix licence link - points to correct repo now.
@wasabrot
Copy link
Contributor Author

wasabrot commented May 5, 2024

Ok, what about the aspect of the license? @wasabrot

Hi @PieWol - we added inline comments to the unit tests. The licence is actually correct and apache 2, but the link in the milestone delivery document pointed to ebics-java-client instead of ebics-java-server. I fixed that in the milestone delivery document. So checkpoints 0a and 0b should be hopefully fine now. Thx a lot, Walter

@PieWol
Copy link
Member

PieWol commented May 7, 2024

Got it, thanks @wasabrot .
Just fyi it looks like there is a typo in your whitepaper with x being defined twice :o
Bildschirm­foto 2024-05-07 um 11 17 54

Other than that thanks for adding inline documentation retroactively to these tests. I can see by the docker demo that the proving and verifying of receipts works but to be honest I have not quite understood how you manage to do this in a zero-knowledge way and at the same time be able to prove that everything is correct. E.g. the data from the bank was issued for the correct account holder.

I can see that my lack of understanding would be more relevant in the evaluation of the first milestone, yet if somebody from your team would be happy to jump on a quick call and explain it to me I would greatly appreciate it :)

I have updated my evaluation so that the delivery is now accepted. 🎉
As for the call I wish for, how about you send me an invite to [email protected]?

@PieWol PieWol merged commit da48789 into w3f:master May 7, 2024
1 check passed
Copy link

github-actions bot commented May 7, 2024

🪙 Please fill out the invoice form in order to initiate the payment process. Thank you!

@RouvenP
Copy link

RouvenP commented May 27, 2024

hi @wasabrot we sent the payment last Friday

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants