Skip to content

Commit

Permalink
Enable Compatibility with Windows (sigp#2333)
Browse files Browse the repository at this point in the history
## Issue Addressed

Windows incompatibility.

## Proposed Changes

On windows, lighthouse needs to default to STDIN as tty doesn't exist. Also Windows uses ACLs for file permissions. So to mirror chmod 600, we will remove every entry in a file's ACL and add only a single SID that is an alias for the file owner.

Beyond that, there were several changes made to different unit tests because windows has slightly different error messages as well as frustrating nuances around killing a process :/

## Additional Info

Tested on my Windows VM and it appears to work, also compiled & tested on Linux with these changes. Permissions look correct on both platforms now. Just waiting for my validator to activate on Prater so I can test running full validator client on windows.

Co-authored-by: ethDreamer <37123614+ethDreamer@users.noreply.github.com>
Co-authored-by: Michael Sproul <micsproul@gmail.com>
  • Loading branch information
3 people committed May 19, 2021
1 parent 58e52f8 commit ba55e14
Show file tree
Hide file tree
Showing 43 changed files with 607 additions and 179 deletions.
14 changes: 14 additions & 0 deletions .github/workflows/test-suite.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,20 @@ jobs:
run: sudo npm install -g ganache-cli
- name: Run tests in release
run: make test-release
release-tests-windows:
name: release-tests-windows
runs-on: windows-latest
needs: cargo-fmt
steps:
- uses: actions/checkout@v1
- name: Get latest version of stable Rust
run: rustup update stable
- name: Install ganache-cli
run: npm install -g ganache-cli
- name: Install make
run: choco install -y make
- name: Run tests in release
run: make test-release
debug-tests-ubuntu:
name: debug-tests-ubuntu
runs-on: ubuntu-latest
Expand Down
98 changes: 92 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions account_manager/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ slashing_protection = { path = "../validator_client/slashing_protection" }
eth2 = {path = "../common/eth2"}
safe_arith = {path = "../consensus/safe_arith"}
slot_clock = { path = "../common/slot_clock" }
filesystem = { path = "../common/filesystem" }
sensitive_url = { path = "../common/sensitive_url" }

[dev-dependencies]
Expand Down
5 changes: 4 additions & 1 deletion account_manager/src/validator/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
)
.arg(
Arg::with_name(STDIN_INPUTS_FLAG)
.takes_value(false)
.hidden(cfg!(windows))
.long(STDIN_INPUTS_FLAG)
.help("If present, read all user inputs from stdin instead of tty."),
)
Expand All @@ -118,7 +120,8 @@ pub fn cli_run<T: EthSpec>(
let spec = env.core_context().eth2_config.spec;

let name: Option<String> = clap_utils::parse_optional(matches, WALLET_NAME_FLAG)?;
let stdin_inputs = matches.is_present(STDIN_INPUTS_FLAG);
let stdin_inputs = cfg!(windows) || matches.is_present(STDIN_INPUTS_FLAG);

let wallet_base_dir = if matches.value_of("datadir").is_some() {
let path: PathBuf = clap_utils::parse_required(matches, "datadir")?;
path.join(DEFAULT_WALLET_DIR)
Expand Down
5 changes: 4 additions & 1 deletion account_manager/src/validator/exit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
)
.arg(
Arg::with_name(STDIN_INPUTS_FLAG)
.takes_value(false)
.hidden(cfg!(windows))
.long(STDIN_INPUTS_FLAG)
.help("If present, read all user inputs from stdin instead of tty."),
)
Expand All @@ -70,7 +72,8 @@ pub fn cli_run<E: EthSpec>(matches: &ArgMatches, env: Environment<E>) -> Result<
let keystore_path: PathBuf = clap_utils::parse_required(matches, KEYSTORE_FLAG)?;
let password_file_path: Option<PathBuf> =
clap_utils::parse_optional(matches, PASSWORD_FILE_FLAG)?;
let stdin_inputs = matches.is_present(STDIN_INPUTS_FLAG);

let stdin_inputs = cfg!(windows) || matches.is_present(STDIN_INPUTS_FLAG);
let no_wait = matches.is_present(NO_WAIT);

let spec = env.eth2_config().spec.clone();
Expand Down
4 changes: 3 additions & 1 deletion account_manager/src/validator/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
)
.arg(
Arg::with_name(STDIN_INPUTS_FLAG)
.takes_value(false)
.hidden(cfg!(windows))
.long(STDIN_INPUTS_FLAG)
.help("If present, read all user inputs from stdin instead of tty."),
)
Expand All @@ -83,7 +85,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
pub fn cli_run(matches: &ArgMatches, validator_dir: PathBuf) -> Result<(), String> {
let keystore: Option<PathBuf> = clap_utils::parse_optional(matches, KEYSTORE_FLAG)?;
let keystores_dir: Option<PathBuf> = clap_utils::parse_optional(matches, DIR_FLAG)?;
let stdin_inputs = matches.is_present(STDIN_INPUTS_FLAG);
let stdin_inputs = cfg!(windows) || matches.is_present(STDIN_INPUTS_FLAG);
let reuse_password = matches.is_present(REUSE_PASSWORD_FLAG);
let keystore_password_path: Option<PathBuf> =
clap_utils::parse_optional(matches, PASSWORD_FLAG)?;
Expand Down
4 changes: 3 additions & 1 deletion account_manager/src/validator/recover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
)
.arg(
Arg::with_name(STDIN_INPUTS_FLAG)
.takes_value(false)
.hidden(cfg!(windows))
.long(STDIN_INPUTS_FLAG)
.help("If present, read all user inputs from stdin instead of tty."),
)
Expand All @@ -86,7 +88,7 @@ pub fn cli_run(matches: &ArgMatches, validator_dir: PathBuf) -> Result<(), Strin
let first_index: u32 = clap_utils::parse_required(matches, FIRST_INDEX_FLAG)?;
let count: u32 = clap_utils::parse_required(matches, COUNT_FLAG)?;
let mnemonic_path: Option<PathBuf> = clap_utils::parse_optional(matches, MNEMONIC_FLAG)?;
let stdin_inputs = matches.is_present(STDIN_INPUTS_FLAG);
let stdin_inputs = cfg!(windows) || matches.is_present(STDIN_INPUTS_FLAG);

eprintln!("secrets-dir path: {:?}", secrets_dir);

Expand Down
32 changes: 4 additions & 28 deletions account_manager/src/wallet/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,9 @@ use eth2_wallet::{
PlainText,
};
use eth2_wallet_manager::{LockedWallet, WalletManager, WalletType};
use filesystem::create_with_600_perms;
use std::ffi::OsStr;
use std::fs;
use std::fs::File;
use std::io::prelude::*;
use std::os::unix::fs::PermissionsExt;
use std::path::{Path, PathBuf};

pub const CMD: &str = "create";
Expand Down Expand Up @@ -83,6 +81,8 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
)
.arg(
Arg::with_name(STDIN_INPUTS_FLAG)
.takes_value(false)
.hidden(cfg!(windows))
.long(STDIN_INPUTS_FLAG)
.help("If present, read all user inputs from stdin instead of tty."),
)
Expand Down Expand Up @@ -153,8 +153,7 @@ pub fn create_wallet_from_mnemonic(
let name: Option<String> = clap_utils::parse_optional(matches, NAME_FLAG)?;
let wallet_password_path: Option<PathBuf> = clap_utils::parse_optional(matches, PASSWORD_FLAG)?;
let type_field: String = clap_utils::parse_required(matches, TYPE_FLAG)?;
let stdin_inputs = matches.is_present(STDIN_INPUTS_FLAG);

let stdin_inputs = cfg!(windows) || matches.is_present(STDIN_INPUTS_FLAG);
let wallet_type = match type_field.as_ref() {
HD_TYPE => WalletType::Hd,
unknown => return Err(format!("--{} {} is not supported", TYPE_FLAG, unknown)),
Expand Down Expand Up @@ -237,26 +236,3 @@ pub fn read_new_wallet_password_from_cli(
},
}
}

/// Creates a file with `600 (-rw-------)` permissions.
pub fn create_with_600_perms<P: AsRef<Path>>(path: P, bytes: &[u8]) -> Result<(), String> {
let path = path.as_ref();

let mut file =
File::create(&path).map_err(|e| format!("Unable to create {:?}: {}", path, e))?;

let mut perm = file
.metadata()
.map_err(|e| format!("Unable to get {:?} metadata: {}", path, e))?
.permissions();

perm.set_mode(0o600);

file.set_permissions(perm)
.map_err(|e| format!("Unable to set {:?} permissions: {}", path, e))?;

file.write_all(bytes)
.map_err(|e| format!("Unable to write to {:?}: {}", path, e))?;

Ok(())
}
Loading

0 comments on commit ba55e14

Please sign in to comment.