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

Amendment related to https://github.com/w3f/Grant-Milestone-Delivery/pull/1175 #2305

Merged
merged 1 commit into from
May 22, 2024

Conversation

AltiMario
Copy link
Contributor

@AltiMario AltiMario commented May 14, 2024

Project Abstract

Develop a Tuxedo Web Wallet and Dapp.

Our goal is to showcase the potential of Polkadot using the UTXO paradigm for the end user, leveraging Tuxedo. The project will consist of two key stages:

  • Stage 1: Developing a simple Web Wallet
  • Stage 2: Developing a basic DApp

Grant level

  • Level 1: Up to $10,000, 2 approvals
  • Level 2: Up to $30,000, 3 approvals
  • Level 3: Unlimited, 5 approvals (for >$100k: Web3 Foundation Council approval)

Application Checklist

  • The application template has been copied and aptly renamed (project_name.md).
  • I have read the application guidelines.
  • Payment details have been provided (bank details via email or Polkadot (USDC & USDT) address in the application).
  • The software delivered for this grant will be released under an open-source license specified in the application.
  • The initial PR contains only one commit (squash and force-push if needed).
  • The grant will only be announced once the first milestone has been accepted (see the announcement guidelines).
  • I prefer the discussion of this application to take place in a private Element/Matrix channel. My username is: @_______:matrix.org (change the homeserver if you use a different one)

@github-actions github-actions bot added the admin-review This application requires a review from an admin. label May 14, 2024
Copy link
Contributor Author

@AltiMario AltiMario left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to the evaluation w3f/Grant-Milestone-Delivery#1175

Copy link
Collaborator

@takahser takahser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AltiMario is there any specific reason for skipping the docker deliverable?

@AltiMario
Copy link
Contributor Author

hi @takahser in milestone 1 we included a docker deliverable.
The original plan was to have 2 web components, we pivoted one of these components by integrating Talisman, which made the dockerization unnecessary.

@PieWol
Copy link
Member

PieWol commented May 17, 2024

Hey @AltiMario , I'd appreciate it if you could provide me some answers to the comments I raised. Thanks.

@PieWol
Copy link
Member

PieWol commented May 17, 2024

I'm expressing some urgency, as this PR is a prerequisite to continuing the milestone evaluation. @AltiMario

@AltiMario
Copy link
Contributor Author

sure thing @PieWol I will write it in a few minutes. Thanks

@AltiMario
Copy link
Contributor Author

Hi @PieWol sorry for the delay in my reply.
I think I covered all queries, for further doubts please don't hesitate to contact me.

1- "Is it correct that you have, instead of creating your own web-based wallet, integrated Talisman to be usable for Tuxedo and your dApp?"

Yes. We pivoted this part of the project following the suggestion of Joshua Orndorff. I added some extra info in the "Additional Information" section of milestone 1.

2- "Please adjust some links of this milestone to better fit the original application and its specifications for the deliverables. Right now the wallet readme isn't even mentioned in the documentation deliverable. Even though I think this is the key documentation for this milestone."

I completely updated the README file to better describe the changes done, but I forgot to add it to the file of milestone 1. I'm going to add the link.

3- "The demo for showcasing the Talisman wallet compatibility seems cut short. In the readme only a 4 second clip is visible."
Fixed.

4- "We will provide [...] a basic tutorial that explains how a user can use the web wallet app for Tuxedo (Private key configuration, local URI to be used to connect to blockchain) and send test transactions, tuxedo balance, which will show how the new functionality works.
Is this all covered by the readme within the wallet directory?"

Because it is not needed a tutorial of how to use Talisman we haven't done it. However, we created a basic video tutorial (3 parts) on how to use the extended CLI-wallet.

5- "Was listed as the specification for this deliverable. Yet in the milestone delivery you are linking to a little showcase video that walks through an offer creation and buying process. No info on private key configuration or how information from the chain is retrieved. It's just a video of a frontend usage."

The Talisman integration doesn't cover the private key configuration but the extended CLI-wallet does.

6- "Is there no written article at all?", "Especially interesting could be a written article that covers your changes to Talisman to be Tuxedo compatible on a high level."

Sorry, I didn't link them properly. Now you can see it in the README and in milestone 1 (section documentation and article). We produced documentation for: APIs spreadsheet, Build and run the project, DApp specification, Frontend development libs, Game design, Talisman wallet, Wireframes, Comparative analysis "CryptoKitties Development: A comparative analysis between EVM - Cardano - Tuxedo/Polkadot"

7- "Furthermore what exactly are you referring to by "webservice"? Is this supposed to be your dApp?"
The solution/architecture is described in the doc "DApp-specification" https://github.com/mlabs-haskell/TuxedoDapp/wiki/DApp-specification documentation. The dockerization for the Talisman integration is not needed. The extended CLI-wallet is an extension of the Tuxedo framework, it can be run in the same way. What is new is the webservice part, where the docker container is provided.

@PieWol
Copy link
Member

PieWol commented May 17, 2024

@AltiMario ,
got it. Thanks!

Finally, what is the work you did to make Talisman work with Tuxedo? I thought Tuxedo / UTXO wasn't supported. Is there any code you had to modify on Talismans side or are you somehow making this work with blind signing? If you can please provide me a link to the code.

@AltiMario
Copy link
Contributor Author

hi @PieWol we used the blind signing, no work was done on the Talisman codebase.
The signing (redeemer creation) for the webservice part is here https://github.com/mlabs-haskell/Tuxedo/blob/cf02ebc8cba89d398e4fc1016ee76b17d764bcd4/webservice-wallet/src/kitty.rs#L937
and for the frontend part is here https://github.com/mlabs-haskell/TuxedoDapp/blob/6f9def9ee2f654762f433c8605b5ba4132c272a2/src/api/utils.ts#L89
For further technical questions related to the core, cli-wallet and the webservice part I invite @NadigerAmit to answer you and @philoniare can address the frontend part.
Both developers are already on other projects but they are available to support the technical conversation in detail, explaining what they did.

@keeganquigley keeganquigley added amendment This PR proposes changes to an existing application. and removed admin-review This application requires a review from an admin. labels May 20, 2024
@PieWol
Copy link
Member

PieWol commented May 21, 2024

Hey @AltiMario ,
thanks again for your feedback. Would you also please answer the comments I made in my review of this very PR, additionally to the points I raised in my evaluation? You should find these in this thread if you scroll up.

@PieWol PieWol added the changes requested The team needs to clarify a few things first. label May 21, 2024
@PieWol
Copy link
Member

PieWol commented May 21, 2024

@AltiMario ,
With you pivoting from creating the web-wallet to simply using Talisman blind signing this seems to be less work than previously outlined. Thus you need to lower the cost of the first milestone retroactively. Would you please come up with a reasoned proposal for this? Thanks!

@AltiMario
Copy link
Contributor Author

hi @PieWol about your comment #2305 (comment) I don't know what I missed in this reply #2305 (comment)
Could you please rephrase your concern or point out what hasn't been properly explained that deserves more clarification?

@AltiMario
Copy link
Contributor Author

@PieWol I think there is a misunderstanding of the PR amendment scope. It is not for reduced work or just for the Talisman integration. It is also for the CLI-wallet extension, which took @NadigerAmit almost 2 months to build. It is a lot of work, probably a bit hidden (?), but that was a mandatory thing to do in order to have the DApp working. We exposed the functions built for the CLI-wallet as APIs of the webservice.
In this project, saving money was not on the agenda. In fact, the resources employed were twice what I initially planned (you can see it in the code contributors list), leading to a significant budget overrun. For MLabs this represented an investment in this technology/framework, with the goal (and hope) to bring further developments and work.
For every question related to the huge amount of work done for the extended CLI-wallet please refer to @NadigerAmit who will be happy and proud to answer and drive you on his code.
Thanks, Mario

Copy link
Member

@PieWol PieWol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @AltiMario ,
please excuse my misunderstanding. I was under the false impression that the cli-wallet extension was already part of the application previous to the amendment. This amendment looks good to me. Thanks for your swift replies and I'm sorry for the inconvenience.

applications/TuxedoDapp.md Show resolved Hide resolved
applications/TuxedoDapp.md Show resolved Hide resolved
applications/TuxedoDapp.md Show resolved Hide resolved
applications/TuxedoDapp.md Show resolved Hide resolved
applications/TuxedoDapp.md Show resolved Hide resolved
@PieWol PieWol removed the changes requested The team needs to clarify a few things first. label May 22, 2024
@semuelle semuelle merged commit 018afb4 into w3f:master May 22, 2024
22 of 23 checks passed
@AltiMario
Copy link
Contributor Author

Hi @PieWol, don't worry, it didn't cause any inconvenience. I'm glad the work has been approved. We have a common goal, to create value for this ecosystem.
Thanks to you and the rest of your team for the support.

@AltiMario AltiMario deleted the tuxedo-amendment branch July 16, 2024 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
amendment This PR proposes changes to an existing application.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants