AleoNet / snarkOS

A Decentralized Operating System for ZK Applications
http://snarkos.org
Apache License 2.0
4.1k stars 2.57k forks source link

[Fix] Fix node start not respecting --bft unless in dev mode #3271

Closed Meshiest closed 4 weeks ago

Meshiest commented 1 month ago

Motivation

  1. An expected behavior of the --bft flag is to allow changing the bound IP and port
  2. This change enables a single PC to run testnet, canary, and mainnet nodes in parallel without resorting to virtualization
  3. This change provides multihome support to snarkOS. Some servers may have more than one public IP address and a user may desire to bind to only one of those addresses.

Test Plan

We have been running validators with modified BFT ports without issue. This change does not add new features to snarkOS

Related PRs

n/a

vicsn commented 1 month ago

This will "break" the existing hacky approach to identify whether we're in --dev mode: https://github.com/AleoNet/snarkOS/pull/3221/files#diff-8fdb42b053ba1205fee0fc78a878297649924dbed64970c52670294c9cd93a2fL327

This is fixed in https://github.com/AleoNet/snarkOS/pull/3221

However, Christian is out of office at the moment. Do you mind squashing/cherry-picking commits from that PR into this PR, so we're sure this gets updated in one go?

Also, historically, we've just noticed changing flags breaks people's existing workflows and its hard to think of everyone depending on this. So please take an extra minute to test with an without the updated flags to see if its working as intended 🙏

Meshiest commented 1 month ago

I was able to verify the bind changes were functional with the following commands (running 4 validators locally) by checking ss -tulpn for the expected bound socket addresses

devnet with bft rebinding:

./target/debug/snarkos start --network 0 --validator --nodisplay --bft 127.0.0.1:5111 --dev 0 --no-dev-txs --dev-num-validators 4 --validators 127.0.0.1:5111,127.0.0.1:5112,127.0.0.1:5113,127.0.0.1:5114
./target/debug/snarkos start --network 0 --validator --nodisplay --bft 127.0.0.1:5112 --dev 1 --validators 127.0.0.1:5111,127.0.0.1:5112,127.0.0.1:5113,127.0.0.1:5114
./target/debug/snarkos start --network 0 --validator --nodisplay --bft 127.0.0.1:5113 --dev 2 --validators 127.0.0.1:5111,127.0.0.1:5112,127.0.0.1:5113,127.0.0.1:5114
./target/debug/snarkos start --network 0 --validator --nodisplay --bft 127.0.0.1:5114 --dev 3 --validators 127.0.0.1:5111,127.0.0.1:5112,127.0.0.1:5113,127.0.0.1:5114

devnet without bft rebinding:

./target/debug/snarkos start --network 0 --validator --nodisplay --dev 0 --no-dev-txs --dev-num-validators 4 --validators 127.0.0.1:5000,127.0.0.1:5001,127.0.0.1:5002,127.0.0.1:5003
./target/debug/snarkos start --network 0 --validator --nodisplay --dev 1 --validators 127.0.0.1:5000,127.0.0.1:5001,127.0.0.1:5002,127.0.0.1:5003
./target/debug/snarkos start --network 0 --validator --nodisplay --dev 2 --validators 127.0.0.1:5000,127.0.0.1:5001,127.0.0.1:5002,127.0.0.1:5003
./target/debug/snarkos start --network 0 --validator --nodisplay --dev 3 --validators 127.0.0.1:5000,127.0.0.1:5001,127.0.0.1:5002,127.0.0.1:5003

validator that is obviously not part of the committee with rebinding:

./target/debug/snarkos start --network 1 --validator --nodisplay --bft 127.0.0.1:5111 --private-key APrivateKey1zkp7QWqPSmrK4durW7Y6uwpbnubWtFGJeYoXtN4jaAPFJmv

validator that is obviously not part of the committee without rebinding:

./target/debug/snarkos start --network 1 --validator --nodisplay --private-key APrivateKey1zkp7QWqPSmrK4durW7Y6uwpbnubWtFGJeYoXtN4jaAPFJmv
vicsn commented 1 month ago

@zosorock can you run CI (perhaps on a mirror branch on AleoNet) since this is not a Provable branch?

vicsn commented 1 month ago

Was able to run CI on this. @Meshiest would you mind merging in ProvableHQ/snarkOS/feat/optional-rest-ip again? Added one commit which removes a failing new unit test which was very incomplete still

Meshiest commented 1 month ago

@vicsn done