Skip to content

Conversation

charlieroth
Copy link

@charlieroth charlieroth commented Feb 1, 2025

This is simply a proposal and none of this code has been manually tested

Motivation

Based on the "tools" that Go offers such as the the internal folder (https://www.bytesizego.com/blog/golang-internal-package), some "best practices" I have picked up while writing Go professionally and learned from long time Go developers such as Bill Kennedy (https://www.youtube.com/watch?v=bQgNYK1Z5ho) I would like to propose an updated structure for this project.

I view this project not only as a great tool to increase sovereignty on the Nostr network but also as an educational opportunity for Go developers interested in contributing to the Nostr network.

The goals of this re-structuring are the following:

  • Remove global variables
  • Remove the usage of the init.go file
  • Prevent data races via sync.Mutex for data structures that are potentially mutated inside multiple go routines
  • Improve modularity and improving testability

Below are my personal thoughts/opinions and the practical actions I have taken to accomplish these goals

Remove Global Variables & init.go File

In general, global variables are a code smell and should be avoided. Of course there are cases where global variables are not an issue. In the case of this codebase I found no situation where global variables were necessary therefore I was able to remove them using other patterns such as dependency injection and data encapsulation in structures.

I removed the init.go file and placed the initRelays function in main.go. The reasoning for this due to the removal of the global variables defined in init.go. Now that the initRelays function has all of it's dependencies passed as arguments, which are created in func main(), it made since to colocate this function in main.go.

Improve Modularity & Testability

Currently the codebase has one package, main, and given the current stage of development, this makes sense. If this project wants to expand in scope or enable relay operators to run some of the components of this project in isolation, such as running a one-off import, modularity will need to improve. The way to improve modularity in a Go codebase is via the package system.

The codebase also lacks tests which again, given it's current stage of development is completely understandable. If a goal of this project is to be software that private relay operators trust with their personal data then having code that is verifiably working is a must. As Kent Beck says, "make it work, make it right, make it fast". Currently Haven works and this is a great accomplishment, now might be a good time to start thinking about making it "right", whatever that means for this project. In my opinion, a logical step in making it "right" is to be the code testable. Making Go code testable first requires the application to be structure in a modular fashion with clear boundaries/definitions on dependencies.

Move towards this goal of modularity, I have structured the different components of this application (as I see them) into Go packages. This is done with mindset of being able to test these different components with minimal dependency on one another and when necessary, with mockable dependencies, in the future. This is how I see Go projects structured throughout the industry so I don't think I am proposing anything out-of-bounds by saying this.

For example, the config package has great influence on other components of this application. If a relay operator incorrectly configures their environment variables the application could attempt to do a database backup to GCP, AWS, etc. without the necessary environment variables being defined, resulting in undesired behavior or crashing the application. Ideally, the config package has logic and tests that ensure that given a particular configuration, the correct application behavior will happen and in the case of invalid configuration the relay operator is able to debug what is incorrectly configured as soon as possible in the startup of the application.

Prevent Data Races via sync.Mutex

With the introduction of the wot package, which defines a WoT structure to hold the state of the Web-of-Trust used in the application, I added a mutex lock to prevent potential data races when mutating the underlying state. In general, if a data structure is mutated in a go routine it is a good idea to add a mutex lock to prevent unwanted data races. While the logic of the program may not actually have any potential for data races of this state it is better to be safe than sorry.

I have applied the above logic to the importr package as well.

@charlieroth charlieroth force-pushed the charlieroth/project-structure-proposal branch 2 times, most recently from 12f438e to 1137769 Compare February 2, 2025 10:51
@charlieroth charlieroth force-pushed the charlieroth/project-structure-proposal branch from 1137769 to 1c07019 Compare February 2, 2025 11:19
@barrydeen
Copy link
Contributor

I appreciate your work on this and the thinking behind it. However I just can't see Haven being so big that it requires seperate packages and modules for each relay type, and I have other relays if people want to run one-off type relays. Haven is supposed to be an all in one, and i think the codebase should reflect that.

However I do like your changes with adding a mutex and making the config cleaner.

@charlieroth
Copy link
Author

I appreciate your work on this and the thinking behind it. However I just can't see Haven being so big that it requires seperate packages and modules for each relay type, and I have other relays if people want to run one-off type relays. Haven is supposed to be an all in one, and i think the codebase should reflect that.

However I do like your changes with adding a mutex and making the config cleaner.

Understood, was just having some fun and hacking around. I will close this and open PRs for the adding mutexes where needed and the config changes

@charlieroth charlieroth closed this Feb 5, 2025
charlieroth pushed a commit to charlieroth/haven that referenced this pull request Feb 6, 2025
- Separate structures for different DB backup methods (AWS, GCP, Generic S3)
- Rename references to the DigitalOcean Spaces backup method to S3, indicating compatibility with any S3-compatible storage
- Update `README.md` and `.env.example` to reflect code changes
ChadFarrow pushed a commit to ChadFarrow/haven that referenced this pull request Jul 25, 2025
- Separate structures for different DB backup methods (AWS, GCP, Generic S3)
- Rename references to the DigitalOcean Spaces backup method to S3, indicating compatibility with any S3-compatible storage
- Update `README.md` and `.env.example` to reflect code changes
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.

2 participants