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.
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
Clean up argument parsing. #5
Conversation
Let's merge each change as a separate commit. Let's use the rustfmt.toml rules from the long discussion about fmt rules in rust-bitcoin to stay consistent with that community |
src/main.rs
Outdated
// the remaining argument is the wallet amount (if any) | ||
let mut wallet_amount = None; | ||
|
||
// parse scheduled channel arguments: pairs of (addr, amount) | ||
let channels = match args.get(1..) { | ||
Some(args) => args | ||
.chunks(2) | ||
.filter_map(|chunk| match chunk.len() { | ||
// parse scheduled channel arg: a pair of (addr, amount) | ||
2 => Some(ScheduledChannel::from_args(&chunk[0], &chunk[1])), // p2p_address, btc_amount | ||
// parse wallet amount (optional) | ||
1 => { | ||
wallet_amount = Some( | ||
bitcoin::Amount::from_str_in( | ||
chunk[0].as_str(), | ||
bitcoin::Denomination::Satoshi, | ||
) | ||
.map_err(ArgError::InvalidWalletAmount), | ||
); | ||
None | ||
} | ||
unexpected_len => panic!("chunk len should only be 1 or 2, got {}", unexpected_len), | ||
}) | ||
.collect::<Result<VecDeque<_>, _>>()?, | ||
None => return Ok(None), | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the comments that explain what is going on. However I think we should keep the, the original loop + match with args.next()
. There's way less nesting and fewer lines. Can we merge the best of both and keep the comments but use the original logic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DanGould I tried again, this is my new solution. Let me know what you think.
// parse scheduled channel arguments: pairs of (addr, amount)
let mut channels = VecDeque::with_capacity(args.len() / 2);
// the remaining single argument is the wallet amount in satoshis (if any)
let wallet_amount = loop {
match (args.next(), args.next()) {
// we have a pair of arguments, interpret it as a scheduled channel (p2p addr, amount)
(Some(addr), Some(amount)) => {
channels.push_back(ScheduledChannel::from_args(addr, amount)?);
}
// if there is a remaining single argument, it is the wallet amount
(Some(amount), None) => {
break bitcoin::Amount::from_str_in(amount, bitcoin::Denomination::Satoshi)
.map_err(ArgError::InvalidWalletAmount)?
}
// if there is no remaining single argument, the wallet amount is 0
_ => break bitcoin::Amount::ZERO,
}
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
beautiful!
557e800
to
3616c11
Compare
9efdee7
to
a0120d7
Compare
use bitcoin::util::address::Address; | ||
use bitcoin::util::psbt::PartiallySignedTransaction; | ||
use bitcoin::{Script, TxOut}; | ||
use hyper::service::{make_service_fn, service_fn}; | ||
use hyper::{Body, Method, Request, Response, Server, StatusCode}; | ||
use ln_types::P2PAddress; | ||
use std::collections::HashMap; | ||
use std::collections::{HashMap}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rustfmt commit can fix this non-change
Cleans up argument parsing and
cargo fmt
setup.