Read general account files at genesis#1624
Conversation
This reverts commit a89b4c9.
| .map(|file_path| { | ||
| let toml_str = fs_err::read_to_string(file_path)?; | ||
| GenesisConfig::read_toml(toml_str.as_str()).with_context(|| { | ||
| GenesisConfig::read_toml_file(file_path).with_context(|| { |
There was a problem hiding this comment.
This swallows some errors, i.e. missing, access rights
There was a problem hiding this comment.
I really don't like this.
The previous variant decoupled parsing from file IO, if we need the error context, then we should use something like:
let reader = fs_err::OpenOptions::new().read(true).open(path)?:
let gc = GenesisConfig::parse(reader).with_context(|| { file.path.display().to_string() })?;
..
There was a problem hiding this comment.
Good point, I refactored things so that:
read_toml_filereads the contents of the providedpathinto a str, and then:- determines the parent dir
- calls into
read_toml, which parses TOML intoGenesisConfig
- then, the caller uses
GenesisConfig::into_state()to construct theGenesisState
LMK if this is what you had in mind, or would you further separate not just parsing of GenesisConfig -> GenesisState but also of path to TOML content?
| #[error( | ||
| "the defined asset {symbol:?} has no corresponding faucet, or the faucet was provided as an account file" | ||
| )] |
There was a problem hiding this comment.
I don't understand how the faucet being specified as an account file is an error, but I'll return to this once I've read the rest.
There was a problem hiding this comment.
Why are generating them if the accounts are checked-in? As a general rule build.rs should be used for out of tree code generation, not to generate code that gets checked in.
There was a problem hiding this comment.
Out of scope: We should revisit how we deal with protobuf generated code too.
Co-authored-by: Mirko <48352201+Mirko-von-Leipzig@users.noreply.github.com>
| // SAFETY: TokenSymbol guarantees valid format, so to_string should not fail | ||
| let raw = symbol.to_string().expect("TokenSymbol should always produce valid string"); |
There was a problem hiding this comment.
I'm actually surprised the method is fallible then. Looking at its code, it really shouldn't be returning an error.
| let file_loaded_accounts = account_entries | ||
| .into_iter() | ||
| .map(|acc| { | ||
| let full_path = config_dir.join(&acc.path); |
There was a problem hiding this comment.
What happens if the account path was specified as an absolute path?
There was a problem hiding this comment.
not supported ATM
3 participants
Changes:
NativeFaucet->NativeFaucetConfig, and make it an enum withParametersandAccountvariantsGenesisConfig::read_toml(toml_content)toread_toml_path(toml_path),Pathinstead, so we can lookup the directory and read in account files accordingly, which are referenced relative to the config file02-with-account-files.toml.macfiles that correspond to AggLayer accounts. The.macfiles are also committedbuild.rsfile inconfig::storemodule that auto-generates theseAccountFilescloses #1429