search

seraphis-migration / wallet3

Info and discussions about a hypothetical full 'wallet2' rewrite from scratch
MIT License
14 stars 0 forks source link

Organizing functionality #29

Open UkoeHB opened 1 year ago

UkoeHB commented 1 year ago

This thread is for discussing 'where stuff should go'.

I propose organizing the low-level code as follows. The important guiding principle is no circular dependencies between directories (between translation units within directories is harder to achieve so I'll leave that as 'nice to have').

One-horse-wagon commented 1 year ago

Is it possible to clone the present Monero repository, dismantle it, and start building and testing a new one, beginning with Tier 1? That would be using the power of Git and GitHub would it not?

UkoeHB commented 1 year ago

@One-horse-wagon the existing codebase is more-or-less organized like this. I am just thinking about how to fit seraphis pieces in there. The biggest thing for me right now is splitting out seraphis crypto so I can do seraphis_crypto -> multisig -> seraphis_core without any circular dependencies.

DangerousFreedom1984 commented 1 year ago

I think it would be nice if you could make more folders in the src/seraphis directory to organize things and I'm in favor of adding stuff there whenever possible instead of adding seraphis stuff in other places. For example, I'm pretty sure that we will need to have equivalent file/functions like these in src/cryptonote_basic/account. and src/cryptonote_basic/cryptonote_basic. and we could create something similar like src/seraphis/seraphis_basic/

One-horse-wagon commented 1 year ago

That's a good idea about folder organization. Bring it up at the next meeting.

On 04.11.2022 15:23, DangerousFreedom1984 wrote:

I think it would be nice if you could make more folders in the src/seraphis directory to organize things and I'm in favor of adding stuff there whenever possible instead of adding seraphis stuff in other places. For example, I'm pretty sure that we will need to have equivalent file/functions like these in src/cryptonote_basic/account. and src/cryptonote_basic/cryptonote_basic. and we could create something similar like src/seraphis/seraphis_basic/

-- Reply to this email directly, view it on GitHub [1], or unsubscribe [2]. You are receiving this because you were mentioned.Message ID: @.***>

Links:

[1] https://github.com/seraphis-migration/wallet3/issues/29#issuecomment-1303629770 [2] https://github.com/notifications/unsubscribe-auth/AGS3CFQN3H5AFA2QXXDFKBDWGUL47ANCNFSM6AAAAAARVOJCYI

rbrunner7 commented 1 year ago

I was thinking into a different direction: If we had separate directories for the Seraphis library (now /src/seraphis) and one new top-level directory for the Seraphis wallet (I propose /src/seraphis_wallet), things should look quite clear e.g. when merging. Also it would be clear that for the time being everything in the Seraphis library folder is @UkoeHB 's exclusive department, and would by convention / agreement need a PR to him to change.

Thus not "Seraphis stuff in a lot of other places" which I wouldn't agree with either, but two neatly separated places for the two parties with different focus and different responsibilities, i.e. only 1 more than now. The question "Is it library, or is it wallet?" would be trivial to answer, just check the top-level folder it's in.

UkoeHB commented 1 year ago

I think it would be nice if you could make more folders in the src/seraphis directory to organize things and I'm in favor of adding stuff there whenever possible instead of adding seraphis stuff in other places. For example, I'm pretty sure that we will need to have equivalent file/functions like these in src/cryptonote_basic/account. and src/cryptonote_basic/cryptonote_basic. and we could create something similar like src/seraphis/seraphis_basic/

I'm generally skeptical about nesting directories. Even though they aid organization, it can get tedious when navigating around. Plus, putting everything seraphis related in one directory implies more coupling between the seraphis sub-directories than actually makes sense. Seraphis crypto is more related to src/crypto than to seraphis mockups, for example.

Thus not "Seraphis stuff in a lot of other places" which I wouldn't agree with either, but two neatly separated places for the two parties with different focus and different responsibilities, i.e. only 1 more than now.

I disagree with this. The current seraphis directory has way too many files and should be pared down. There is also not a good place for seraphis wallet tool mockups to go other than in-line in the wallet tool directory.

rbrunner7 commented 1 year ago

Do I understand correctly, @UkoeHB : You don't oppose a new and separate folder for the Seraphis wallet, you oppose having only two folders with Seraphis stuff?

Thinking again, I wouldn't really mind more than two such folders, but I am like you sceptical that it's a good idea to nest, and quite in general hesitating to put wallet-related code into a directory that should be "your turf" for the time being.

UkoeHB commented 1 year ago

Thinking again, I wouldn't really mind more than two such folders, but I am like you sceptical that it's a good idea to nest, and quite in general hesitating to put wallet-related code into a directory that should be "your turf" for the time being.

Yes the wallet stuff should go in at least one new directory: src/seraphis_wallet_tools for wallet components (these are implementations - tied to the daemon, file system, etc.) and src/seraphis_mocks for wallet tool mockups (a catch-all mock-ups directory). It may be possible to then make wallet objects (src/seraphis_default_wallets maybe?) as smallish composites of wallet tools, which in turn call down into src/seraphis_core. Then on top of wallet objects are wallet executables that use the wallet object APIs in different ways (e.g. the CLI or GUI executables).

rbrunner7 commented 1 year ago

IHMO definitely a subject to discuss in a meeting as well.

/src has right now around 30 top-level folders, with some folders having quite some number of files and sub-folders in it. If we start to add a significant number of Seraphis related folders at the top level, to fully avoid any nesting, I am not sure other devs will be really enthusiastic ...

Something like /src/seraphis_wallet/tools or /src/seraphis_wallet/mocks does not yet look too daunting to me - if it stays at two levels down.

UkoeHB commented 1 year ago

Right now I have the seraphis_lib branch code organized into these directories: seraphis_crypto, seraphis, seraphis_mocks. This separation is working well. The seraphis directory may benefit from further splitting up, but for now I am leaving it alone.