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

Necessary conditions for robustness and portability #1

Open
spirobel opened this issue Dec 5, 2022 · 22 comments
Open

Necessary conditions for robustness and portability #1

spirobel opened this issue Dec 5, 2022 · 22 comments

Comments

@spirobel
Copy link

spirobel commented Dec 5, 2022

To increase the robustness and portability of wallet3 it should be built around a purely functional library that meets the following conditions:

  1. C ABI
  2. No concurrency and networking mixed in.
  3. No hidden assumptions about the environment.
  4. No side effects and hidden control flows.
  5. No unnecessary dependencies sprinkled in (for example boost)
  6. Minimal amount of code paths per function.

One of the lessons to learn from wallet2:
instead of creating one very tightly integrated piece of software that tries to solve all of these things at once, the following issues should be dealt with by the wallet implementers:

  • concurrency (respond to user input and not stall)
  • networking (communicating with the daemon)
  • parallelism (threading)
  • logging

The side effect of leaving these problems to implementers is that there will be a better separation of concerns. There should be no full implementation of a wallet that does these things in the monero core repo.

We need to avoid at all costs that the example wallet library is turned again into the de facto standard wallet implementation.

We should make a hard rule that no code gets checked into the main monero repo that breaks these rules:

  1. "example" wallet code that touches the concerns mentioned above
  2. code that breaks the 6 conditions necessary for robust and portable wallet code.

I invite you to read the custom vendored easylogging_c++ library that gets shipped with Monero:
https://github.com/monero-project/monero/blob/master/external/easylogging%2B%2B/easylogging%2B%2B.cc

This library introduces a dependency on signal.h even for code that should be strictly functional and without side effects.
This is an example for 3 issues that we need to avoid:

  1. Code with side effects that serve no purpose
  2. Dubious unnecessary dependencies
  3. Creeping hidden assumptions about the environment the code is running in

Ask yourself this: do you want the code that handles your money contain a 4000 line library to print strings to a console?
(in many different colors and formats)

@spirobel spirobel changed the title Necessary conditions to increase robustness and portability Necessary conditions for robustness and portability Dec 5, 2022
@rbrunner7
Copy link
Member

rbrunner7 commented Dec 5, 2022

We need to avoid at all costs that the example wallet library is turned again into the de facto standard wallet implementation

If this is an important center argument of your issue, I have to say I don't understand it, and I don't see how that would follow from the rest of what you write.

What is, at its core, the big problem you see with a "de facto standard wallet implementation"? A problem so big and so grave that we have to avoid it "at all cost"?

This workgroup here currently has the express top-level goal of implementing exactly that, a standard wallet implementation that most wallet apps can use, at least as I currently understand it, so seems to me your argument can in theory have very wide repercussions.

By the way, did you already look closer at @UkoeHB 's Seraphis library? I am not sure, but it could be that library is already in large parts what you wish for with your points 1. to 6.

@sausagenoods
Copy link

I also agree that wallet3 should implement minimal wallet functions. A higher level program should deal with logging, parallelism and concurrency.

@spirobel
Copy link
Author

spirobel commented Dec 5, 2022

I also agree that wallet3 should implement minimal wallet functions. A higher level program should deal with logging, parallelism and concurrency.

Great to see more people share this view!

This workgroup here currently has the express top-level goal of implementing exactly that, a standard wallet implementation that most wallet apps can use,

you are repeating the same mistake again. Just provide a library for all the cryptography with a C ABI. Keep it purely functional. The definition of the API and the implementation of the wallets can not be done by the same person.
Otherwise everything will be entangled.

By the way, did you already look closer at @UkoeHB 's Seraphis library? I am not sure, but it could be that library is already in large parts what you wish for with your points 1. to 6.

Just to clarify, you mean this code, right? https://github.com/UkoeHB/monero/tree/seraphis_lib/src/seraphis
It fails the requirements. It does not provide a C ABI. It has boost dependencies. Again: this already looks very much like wallet2.
Create an API and explain it to others. Don't try to do everything as a single person. Otherwise everything will be entangled.
We should have multiple teams working on this instead of just letting one person create a giant monolith that is then dependent on this one person.

@rbrunner7
Copy link
Member

you are repeating the same mistake again

You can of course just claim that something is a mistake without giving arguments that other people can read, judge, form their own opinion on, and comment, but I am pretty sure that this will not be very successful. If you claim this workgroup (not merely I myself, I am not alone, we are a workgroup) commits a mistake, please state what the mistake is in your opinion, and most importantly argue why it's a mistake.

It fails the requirements. It does not provide a C ABI.

Monero is implemented in C++. Thus also @UkoeHB 's Seraphis library, because it's meant to become an integral part of the Monero codebase, is written in C++. Providing a C ABI would be a fundamental change of direction. Without very good arguments why that Seraphis library should be rewritten to offer a C ABI you won't get anywhere IMHO. Just claiming "it fails the requirements" doesn't cut it.

@spirobel
Copy link
Author

spirobel commented Dec 5, 2022

Providing a C ABI would be a fundamental change of direction.

No it would not be. You can wrap the functions that you want to expose as an API as extern "C".

The cryptographic purely functional components can also be written as pure C. That would definitively not be "a fundamental change in direction".

without giving arguments

just to make sure we are on the same page:
From you point of view, what are the biggest problems with wallet2.cpp that we need to avoid repeating?

@rbrunner7
Copy link
Member

From you point of view, what are the biggest problems with wallet2.cpp that we need to avoid repeating?

I am ready to discuss that anytime, but I think this issue is not the best place to do so. You could ask this question in the Matrix room or the bridged IRC channel, where I think other people would be interested to read my answer and the discussion that may result from it.

You opened a fresh issue here, with proposals what this workgroup should build, and how it should build it, and without good arguments for what I see as quite extraordinary proposals I fail to see the value of this particular issue here.

The cryptographic purely functional components can also be written as pure C

Why should they? Convince me. No need to argue all over the place, for starters we could very well concentrate on this: You seem to put high value on having a pure C interface. Please elaborate.

@UkoeHB
Copy link
Collaborator

UkoeHB commented Dec 5, 2022

Monero development has never exposed a C ABI. Even src/crypto.h wraps the core crypto which is in C into a C++ namespace.

@spirobel
Copy link
Author

spirobel commented Dec 5, 2022

Please elaborate.

Here are three user stories: #2 I don't see how wallet3 in its current form addresses those.

Matrix room or the bridged IRC channel,

Please open another issue here and answer this question. You love to ask others to elaborate. So please do so too. I am unsure if you are aware of the many short comings of wallet2. This question is highly relevant for making sure the quality and UX of Monero software improves. We need to have a discussion about this and we can't let this get buried in some IRC chat log.

Convince me.

What is your official title with this workgroup by the way?

@sneurlax
Copy link

sneurlax commented Dec 5, 2022

One argument for exposing a C ABI is so that other languages like Go and Rust can access wallet fundamentals more easily.

But Monero is responsible for being complete as a standalone project.

Those developers that need features which benefit other languages ought to take it upon themselves to change the code to play with Go, Rust, Dart, etc., more nicely. Monero is in C++ and monero core developers develop in C++. You'll need significant traction/consensus in order to change the development path (which was set over a year ago now and after papers and publications were shared and comments requested) like this. My recommendation is that if you want these changes, start contributing them now, because I don't think the core developers will do this work for you unless there's overwhelming demand

@sneurlax
Copy link

sneurlax commented Dec 5, 2022

Maintaining a fork which is more portable would be a valuable contribution that would probably be merged in and used if it's the only or best way to implement the user stories from your other issue. I'd love to collaborate with you on this issue: I also like to access Monero functions with other languages I prefer more than C++. If you'd like help, let's try and make the code more portable, but my response with regards to this issue is just "let the core C++ developers continue, if you want other languages supported better too, let's work together on that now"

@UkoeHB
Copy link
Collaborator

UkoeHB commented Dec 5, 2022

Aside from 'C ABI' and 'no boost', the seraphis library already satisfies your requested behavior. In fact, you could build your very own wallet using the seraphis library :). At some point though, you run out of things that can be librarized and start needing to develop specific solutions.

@UkoeHB
Copy link
Collaborator

UkoeHB commented Dec 5, 2022

It fails the requirements. It does not provide a C ABI. It has boost dependencies. Again: this already looks very much like wallet2.
Create an API and explain it to others. Don't try to do everything as a single person. Otherwise everything will be entangled.
We should have multiple teams working on this instead of just letting one person create a giant monolith that is then dependent on this one person.

These comments are hard to believe. Have you looked at wallet2? Have you read the seraphis unit tests demoing the library? Do you have qualified C devs lined up to rewrite the library? Have you been following the seraphis research and development that has been going on for the last 15 months? It's hard to see your suggestions as realistic.

@spirobel
Copy link
Author

spirobel commented Dec 6, 2022

In fact, you could build your very own wallet using the seraphis library :)

That would be very hard because nothing is documented at the moment. It is unclear which functions will form a stable API. And there is no C ABI so the interop from most languages is hard.

At some point though, you run out of things that can be librarized and start needing to develop specific solutions.

At that point you have to start documenting and reaching out to others so they can implement the rest. If you continue to implement the rest of the wallet, many assumptions about "what a wallet is" will be hardcoded in the final blob that is delivered after the waterfall development is over.

Do you have qualified C devs lined up to rewrite the library?

There is no need to rewrite everything in C. You can just wrap the functions you want to expose in extern "C" so others can use them as well. Also as you mentioned: many of the crypto functions are already written in C. Just expose them, so that people outside of the CPP world can use this too.

but my response with regards to this issue is just "let the core C++ developers continue, if you want other languages supported better too, let's work together on that now"

How can we work on that, if nothing gets documented and we just ought to accept a giant blob after the waterfall development is over? It is not a big problem to expose a C ABI instead of a CPP API. Wrap the functions in extern "C".

So please @UkoeHB lets talk about the API design. Where is the hurt in exposing a C ABI instead of a cpp API? It would help a lot of people immensely!

I'd love to collaborate with you on this issue: I also like to access Monero functions with other languages I prefer more than C++.

@sneurlax okay lets do this! The user stories are a great point to start!

@UkoeHB
Copy link
Collaborator

UkoeHB commented Dec 6, 2022

At that point you have to start documenting and reaching out to others so they can implement the rest. If you continue to implement the rest of the wallet, many assumptions about "what a wallet is" will be hardcoded in the final blob that is delivered after the waterfall development is over.

One of the advantages of being on IRC is you learn things like 'koe is winding down involvement with Monero and plans to cease development work soon'.

You can just wrap the functions you want to expose in extern "C" so others can use them as well.

There are plenty of C++ types that are also exposed. You are suggesting a pretty serious rewrite. I recommend watching the code walkthrough video.

Also as you mentioned: many of the crypto functions are already written in C. Just expose them, so that people outside of the CPP world can use this too.

Monero is NOT a crypto library, and absolutely should not expose crypto functions for general use.

@spirobel
Copy link
Author

spirobel commented Dec 6, 2022

One of the advantages of being on IRC is you learn things like 'koe is winding down involvement with Monero and plans to cease development work soon'.

I saw this statement in the seraphis paper:
‡Author ‘koe’ worked on this document partly as an employee of MobileCoin, Inc.

So you did this work on Seraphis while working for the company behind MobileCoin. Was this part of your role there or is this work unrelated to your involvement with MobileCoin?

If Seraphis is such a great invention and you sincerely believe Monero should adopt this protocol, why do you want to stop working on it and work elsewhere?

@rbrunner7
Copy link
Member

Maybe think again whether you want to let these questions stand? Even if people here may not follow your ideas, you may still want to work together with them, and I really don't think that this will help. Beside, they seem to be pretty off-topic and quite non-technical to me in this issue.

@UkoeHB
Copy link
Collaborator

UkoeHB commented Dec 6, 2022

@spirobel if you want to have an off-topic conversation, you can join IRC or email me.

@spirobel
Copy link
Author

spirobel commented Dec 7, 2022

you may still want to work together with them

It is not me who wants to leave the project. It is koe. I also don't think these questions are off topic.

The Seraphis paper reads like the protocol was designed for MobileCoin and not for Monero. Here is just one example:
"This optimization would not be useful in a cryptocurrency like MobileCoin where only e-notes and linking tags
are stored in the ledger, and transactions are discarded" https://raw.githubusercontent.com/UkoeHB/Seraphis/master/Seraphis-0-0-16.pdf (page 22)

So I think it is better to address these questions here, because it is not just me who would like to hear an answer to this.
From the answers we got so far, it seems like this project is unaware and unwilling to fix the many issues that wallet2.cpp comes with.
The Seraphis protocol proposal also does not solve the dependency of Monero's privacy guarantees on transaction uniformity and randomness of decoy selection.
So naturally there is the question if it is in the interests of the Monero community to take on the risk that comes with this protocol and codebase.

Why not just address these questions head on so that we can focus on fixing the problems in front of us?

@UkoeHB
Copy link
Collaborator

UkoeHB commented Dec 7, 2022

A remarkable turn toward the dramatic. What you say about a C API/ABI is certainly interesting and possibly useful, assuming the manpower and inspiration to pull it together exists (despite what you claim, building a large C-only interface is not trivial especially when dealing with a 50k+ line project that is thoroughly C++). Aside from that, vaguely claiming "it seems like this project is unaware and unwilling to fix the many issues that wallet2.cpp comes with" is quite questionable considering it isn't clear you've actually read/watched much of the material related to seraphis.

@spirobel
Copy link
Author

spirobel commented Dec 7, 2022

There is nothing vague about what I am saying and the questions I am asking.

@sneurlax
Copy link

sneurlax commented Dec 7, 2022

There are many years worth of discussion and work relating to wallet2 shortcomings

@rbrunner7
Copy link
Member

According to his Twitter @spirobel is building a browser wallet for Monero.

Technically such browser wallets are a quite special use case. I agree with him that it would be easiest to implement web wallets if all the needed Monero core code would have a pure C interface, or even better, if all that code would be written in simple C and fullfil the requirements 1 to 6 listed in the issue: no side effects, best not keeping state themselves about anything, no threads, a minimum of dependencies and there sure nothing as big and complex as Boost.

Just code lying there that you can pick, match and freely combine like pieces of Lego. Simple, logical.

The way I see it, this unfortunately collides with the reality of the Monero codebase. Which, for starters, is written in C++. And which does currently use Boost and other quite big dependencies.

My proposal how to proceed in the light of this: Accept the basic realities of the Monero codebase, and build the wallet along the same lines, but at the same time try to work together with authors of browser wallets to see what still can be done to make their job easier. I personally spoke with people from MyMonero and RINO back in February, as a start.

It's certainly nice when @spirobel talks in one of this tweets about "healthy competition" regarding Monero wallet code development. Healthy competition is, well, healthy. However I did not yet see much of what I would call thus, frankly.

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

No branches or pull requests

5 participants