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.

Sign up for GitHub

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

Jump to bottom

clightning: native database replication #436

Merged
merged 3 commits into from
Aug 13, 2022
Merged

clightning: native database replication #436

merged 3 commits into from
Aug 13, 2022

Conversation

nixbitcoin
Copy link
Member

@nixbitcoin nixbitcoin commented Jan 6, 2022 •
edited
Loading

Not ready for merge until clightning 0.10.3.

Supersedes #415

Coauthor: @erikarvstedt

@nixbitcoin nixbitcoin force-pushed the clightning-backup-nb branch 5 times, most recently from 4902de4 to a8e9b0d Compare January 11, 2022 12:14
jonasnick
jonasnick reviewed Jan 12, 2022
View reviewed changes
Copy link
Member

@jonasnick jonasnick left a comment

Choose a reason for hiding this comment

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

Very cool stuff!

Ideally we would also document how to restrict the capabilities of the nix-bitcoin node on an SSHFS target as much as possible. It should only be able to access the directory and not be able to access or execute anything else.

modules/clightning-db-replication.nix Outdated Show resolved Hide resolved
modules/clightning-db-replication.nix Outdated Show resolved Hide resolved
docs/services.md Show resolved Hide resolved
docs/services.md Outdated Show resolved Hide resolved
modules/clightning-db-replication.nix Outdated Show resolved Hide resolved
@nixbitcoin
Copy link
Member Author

nixbitcoin commented Jan 17, 2022 •
edited
Loading

Ideally we would also document how to restrict the capabilities of the nix-bitcoin node on an SSHFS target as much as possible. It should only be able to access the directory and not be able to access or execute anything else.

We could to this using Subsystem sftp internal-sftp in OpenSSH as detailed here.

EDIT: Documented in d116f6e622a8d0c24cb970dcbd5f39872ee12f2a

@nixbitcoin nixbitcoin force-pushed the clightning-backup-nb branch 3 times, most recently from 2a362d1 to d116f6e Compare January 18, 2022 13:56
jonasnick
jonasnick reviewed Jan 24, 2022
View reviewed changes
docs/services.md Outdated Show resolved Hide resolved
@nixbitcoin
Copy link
Member Author

nixbitcoin commented Jan 24, 2022 via email

@nixbitcoin nixbitcoin force-pushed the clightning-backup-nb branch from d116f6e to 7b0c617 Compare January 25, 2022 15:25
@nixbitcoin nixbitcoin added this to the 0.1.0 milestone Feb 16, 2022
@prusnak
Copy link
Contributor

prusnak commented Apr 27, 2022

Not ready for merge until clightning 0.10.3.

clightning 0.11.0.1 was merged to nixpkgs in NixOS/nixpkgs#170584

erikarvstedt
erikarvstedt reviewed May 14, 2022
View reviewed changes
modules/clightning-db-replication.nix Outdated
in "sqlite3://${mainDB}:${replicaDB}";

systemd.services.clightning = {
bindsTo = mkIf useMounts [ "clightning-replication-mounts.server" ];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
bindsTo = mkIf useMounts [ "clightning-replication-mounts.server" ];
bindsTo = mkIf useMounts [ "clightning-replication-mounts.service" ];

@nixbitcoin
Copy link
Member Author

nixbitcoin commented May 25, 2022 •
edited
Loading

Running backup server in my local network over sshfs with no issues. Haven't tried across the internet over Tor/VPN for fear of latency issues. Ready for testing/merge IMO.

@nixbitcoin nixbitcoin force-pushed the clightning-backup-nb branch from 7b0c617 to 2203031 Compare May 25, 2022 12:18
jonasnick
jonasnick reviewed May 30, 2022
View reviewed changes
Copy link
Member

@jonasnick jonasnick left a comment

Choose a reason for hiding this comment

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

utACK 2203031 mod fixups

@nixbitcoin nixbitcoin force-pushed the clightning-backup-nb branch 2 times, most recently from a051ebf to e086fc5 Compare June 1, 2022 13:01
@nixbitcoin
Copy link
Member Author

Fixups merged and rebased

elsirion
elsirion previously approved these changes Jun 2, 2022
View reviewed changes
Copy link
Contributor

@elsirion elsirion left a comment

Choose a reason for hiding this comment

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

Looking forward to using it 😃

modules/clightning-db-replication.nix Outdated Show resolved Hide resolved
@nixbitcoin
Copy link
Member Author

Can we review/merge this?

elsirion
elsirion reviewed Jul 21, 2022
View reviewed changes
docs/services.md Outdated Show resolved Hide resolved
@nixbitcoin nixbitcoin force-pushed the clightning-backup-nb branch from e086fc5 to bc4485e Compare July 22, 2022 08:19
@nixbitcoin
Copy link
Member Author

Rebased to master

@nixbitcoin
Copy link
Member Author

nixbitcoin commented Jul 22, 2022 •
edited
Loading

Added a test for sshfs client-server replication. Run it with

./run-tests.sh -s clightningReplication

@nixbitcoin nixbitcoin force-pushed the clightning-backup-nb branch from bc4485e to 932a252 Compare July 22, 2022 12:32
@erikarvstedt
Copy link
Collaborator

Fixups.

@nixbitcoin nixbitcoin force-pushed the clightning-backup-nb branch from 932a252 to 775388e Compare August 5, 2022 13:03
@nixbitcoin
Copy link
Member Author

Thanks for the phenomenal review. Let's get this merged @jonasnick!

erikarvstedt
erikarvstedt approved these changes Aug 5, 2022 •
edited
Loading
View reviewed changes
erikarvstedt
erikarvstedt requested changes Aug 5, 2022
View reviewed changes
Copy link
Collaborator

@erikarvstedt erikarvstedt left a comment

Choose a reason for hiding this comment

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

Fixup.

nixbitcoin and others added 3 commits August 5, 2022 13:48
@nixbitcoin
clightning: native database replication
1750783 
Don't put `clightning.replication` options in
`examples/configuration.nix` until it is more "battle-tested."
@erikarvstedt @nixbitcoin
run-tests: support tests with multiple VMs in run
952cd37
@erikarvstedt @nixbitcoin
tests: add comment
5eb0149
@nixbitcoin nixbitcoin force-pushed the clightning-backup-nb branch from 775388e to 5eb0149 Compare August 5, 2022 13:49
erikarvstedt
erikarvstedt approved these changes Aug 5, 2022
View reviewed changes
Copy link
Collaborator

@erikarvstedt erikarvstedt left a comment

Choose a reason for hiding this comment

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

ACK 5eb0149

jonasnick
jonasnick approved these changes Aug 13, 2022
View reviewed changes
Copy link
Member

@jonasnick jonasnick left a comment

Choose a reason for hiding this comment

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

Wow, our first test network. Thanks @erikarvstedt!

ACK 5eb0149

@jonasnick jonasnick merged commit 749ed14 into fort-nix:master Aug 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elsirion elsirion elsirion left review comments

@jonasnick jonasnick jonasnick approved these changes

@erikarvstedt erikarvstedt erikarvstedt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.1.0
Development

Successfully merging this pull request may close these issues.
5 participants
@nixbitcoin @prusnak @erikarvstedt @jonasnick @elsirion