Skip to content

Commit

Permalink
refactor to pass slices around and not &Vec<> (#498)
Browse files Browse the repository at this point in the history
cleaned up a couple of unwrap() calls
  • Loading branch information
antiochp committed Aug 3, 2020
1 parent 7a00337 commit e00c61f
Show file tree
Hide file tree
Showing 9 changed files with 50 additions and 57 deletions.
2 changes: 1 addition & 1 deletion controller/src/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ pub fn txs(
account: &str,
cur_height: u64,
validated: bool,
txs: &Vec<TxLogEntry>,
txs: &[TxLogEntry],
include_status: bool,
dark_background_color_scheme: bool,
) -> Result<(), Error> {
Expand Down
4 changes: 2 additions & 2 deletions impls/src/adapters/slatepack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl<'a> PathToSlatepack<'a> {

pub fn get_slatepack(&self, decrypt: bool) -> Result<Slatepack, Error> {
let data = self.get_slatepack_file_contents()?;
self.packer.deser_slatepack(data, decrypt)
self.packer.deser_slatepack(&data, decrypt)
}
}

Expand Down Expand Up @@ -80,7 +80,7 @@ impl<'a> SlatePutter for PathToSlatepack<'a> {
impl<'a> SlateGetter for PathToSlatepack<'a> {
fn get_tx(&self) -> Result<(Slate, bool), Error> {
let data = self.get_slatepack_file_contents()?;
let slatepack = self.packer.deser_slatepack(data, true)?;
let slatepack = self.packer.deser_slatepack(&data, true)?;
Ok((self.packer.get_slate(&slatepack)?, true))
}
}
12 changes: 6 additions & 6 deletions integration/tests/simulnet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ fn long_fork_test_mining(blocks: u64, n: u16, s: &servers::Server) {
);
}

fn long_fork_test_case_1(s: &Vec<servers::Server>) {
fn long_fork_test_case_1(s: &[servers::Server]) {
println!("\ntest case 1 start");

// Mine state_sync_threshold-7 blocks on s0
Expand Down Expand Up @@ -621,7 +621,7 @@ fn long_fork_test_case_1(s: &Vec<servers::Server>) {
println!("test case 1 passed")
}

fn long_fork_test_case_2(s: &Vec<servers::Server>) {
fn long_fork_test_case_2(s: &[servers::Server]) {
println!("\ntest case 2 start");

// Mine 20 blocks on s0
Expand Down Expand Up @@ -670,7 +670,7 @@ fn long_fork_test_case_2(s: &Vec<servers::Server>) {
println!("test case 2 passed")
}

fn long_fork_test_case_3(s: &Vec<servers::Server>) {
fn long_fork_test_case_3(s: &[servers::Server]) {
println!("\ntest case 3 start");

// Mine cut_through_horizon+1 blocks on s4
Expand Down Expand Up @@ -736,7 +736,7 @@ fn long_fork_test_case_3(s: &Vec<servers::Server>) {
println!("test case 3 passed")
}

fn long_fork_test_case_4(s: &Vec<servers::Server>) {
fn long_fork_test_case_4(s: &[servers::Server]) {
println!("\ntest case 4 start");

let _ = s[1].chain.compact();
Expand Down Expand Up @@ -783,7 +783,7 @@ fn long_fork_test_case_4(s: &Vec<servers::Server>) {
println!("test case 4 passed")
}

fn long_fork_test_case_5(s: &Vec<servers::Server>) {
fn long_fork_test_case_5(s: &[servers::Server]) {
println!("\ntest case 5 start");

let _ = s[1].chain.compact();
Expand Down Expand Up @@ -831,7 +831,7 @@ fn long_fork_test_case_5(s: &Vec<servers::Server>) {
}

#[allow(dead_code)]
fn long_fork_test_case_6(s: &Vec<servers::Server>) {
fn long_fork_test_case_6(s: &[servers::Server]) {
println!("\ntest case 6 start");

let _ = s[1].chain.compact();
Expand Down
10 changes: 5 additions & 5 deletions libwallet/src/api_impl/owner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ where
recipients: vec![],
dec_key: None,
});
let slatepack = packer.deser_slatepack(slatepack.as_bytes().to_vec(), true)?;
let slatepack = packer.deser_slatepack(slatepack.as_bytes(), true)?;
return packer.get_slate(&slatepack);
} else {
for index in secret_indices {
Expand All @@ -181,7 +181,7 @@ where
recipients: vec![],
dec_key: (&dec_key).as_ref(),
});
let res = packer.deser_slatepack(slatepack.as_bytes().to_vec(), true);
let res = packer.deser_slatepack(slatepack.as_bytes(), true);
let slatepack = match res {
Ok(sp) => sp,
Err(_) => {
Expand Down Expand Up @@ -218,7 +218,7 @@ where
dec_key: None,
});
if secret_indices.is_empty() {
packer.deser_slatepack(slatepack.as_bytes().to_vec(), false)
packer.deser_slatepack(slatepack.as_bytes(), false)
} else {
for index in secret_indices {
let dec_key = Some(get_slatepack_secret_key(
Expand All @@ -231,7 +231,7 @@ where
recipients: vec![],
dec_key: (&dec_key).as_ref(),
});
let res = packer.deser_slatepack(slatepack.as_bytes().to_vec(), true);
let res = packer.deser_slatepack(slatepack.as_bytes(), true);
let slatepack = match res {
Ok(sp) => sp,
Err(_) => {
Expand All @@ -240,7 +240,7 @@ where
};
return Ok(slatepack);
}
packer.deser_slatepack(slatepack.as_bytes().to_vec(), false)
packer.deser_slatepack(slatepack.as_bytes(), false)
}
}

Expand Down
4 changes: 2 additions & 2 deletions libwallet/src/slate_versions/v4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,15 +294,15 @@ pub struct TransactionBodyV4 {
pub kers: Vec<TxKernelV4>,
}

fn inputs_are_empty(v: &Vec<InputV4>) -> bool {
fn inputs_are_empty(v: &[InputV4]) -> bool {
v.len() == 0
}

fn default_inputs() -> Vec<InputV4> {
vec![]
}

fn outputs_are_empty(v: &Vec<OutputV4>) -> bool {
fn outputs_are_empty(v: &[OutputV4]) -> bool {
v.len() == 0
}

Expand Down
42 changes: 20 additions & 22 deletions libwallet/src/slatepack/armor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,52 +47,48 @@ pub struct SlatepackArmor;

impl SlatepackArmor {
/// Decode an armored Slatepack
pub fn decode(data: &str) -> Result<Vec<u8>, Error> {
// Convert the armored slate to bytes for parsing
let armor_bytes: Vec<u8> = data.as_bytes().to_vec();
pub fn decode(armor_bytes: &[u8]) -> Result<Vec<u8>, Error> {
// Collect the bytes up to the first period, this is the header
let header_bytes = &armor_bytes
let header_bytes = armor_bytes
.iter()
.take_while(|byte| **byte != b'.')
.cloned()
.collect::<Vec<u8>>();
// Verify the header...
check_header(&header_bytes)?;
// Get the length of the header
let header_len = *&header_bytes.len() + 1;
let header_len = header_bytes.len() + 1;
// Skip the length of the header to read for the payload until the next period
let payload_bytes = &armor_bytes[header_len as usize..]
let payload_bytes = armor_bytes[header_len as usize..]
.iter()
.take_while(|byte| **byte != b'.')
.cloned()
.collect::<Vec<u8>>();
// Get length of the payload to check the footer framing
let payload_len = *&payload_bytes.len();
let payload_len = payload_bytes.len();
// Get footer bytes and verify them
let consumed_bytes = header_len + payload_len + 1;
let footer_bytes = &armor_bytes[consumed_bytes as usize..]
let footer_bytes = armor_bytes[consumed_bytes as usize..]
.iter()
.take_while(|byte| **byte != b'.')
.cloned()
.collect::<Vec<u8>>();
check_footer(&footer_bytes)?;
// Clean up the payload bytes to be deserialized
let clean_payload = &payload_bytes
let clean_payload = payload_bytes
.iter()
.filter(|byte| !WHITESPACE_LIST.contains(byte))
.cloned()
.collect::<Vec<u8>>();
// Decode payload from base58
let base_decode = bs58::decode(&clean_payload).into_vec().unwrap();
let base_decode = bs58::decode(&clean_payload)
.into_vec()
.map_err(|_| ErrorKind::SlatepackDeser("Bad bytes".into()))?;
let error_code = &base_decode[0..4];
let slatepack_bytes = &base_decode[4..];
// Make sure the error check code is valid for the slate data
error_check(&error_code.to_vec(), &slatepack_bytes.to_vec())?;
error_check(error_code, slatepack_bytes)?;
// Return slate as binary or string
/*let slatepack_bin = byte_ser::from_bytes::<SlatepackBin>(&slate_bytes).map_err(|e| {
error!("Error reading JSON Slatepack: {}", e);
ErrorKind::SlatepackDeser
})?;*/
Ok(slatepack_bytes.to_vec())
}

Expand All @@ -107,8 +103,8 @@ impl SlatepackArmor {
}

// Takes an error check code and a slate binary and verifies that the code was generated from slate
fn error_check(error_code: &Vec<u8>, slate_bytes: &Vec<u8>) -> Result<(), Error> {
let new_check = generate_check(&slate_bytes)?;
fn error_check(error_code: &[u8], slate_bytes: &[u8]) -> Result<(), Error> {
let new_check = generate_check(slate_bytes)?;
if error_code.iter().eq(new_check.iter()) {
Ok(())
} else {
Expand All @@ -120,8 +116,9 @@ fn error_check(error_code: &Vec<u8>, slate_bytes: &Vec<u8>) -> Result<(), Error>
}

// Checks header framing bytes and returns an error if they are invalid
fn check_header(header: &Vec<u8>) -> Result<(), Error> {
let framing = str::from_utf8(&header).unwrap();
fn check_header(header: &[u8]) -> Result<(), Error> {
let framing =
str::from_utf8(header).map_err(|_| ErrorKind::SlatepackDeser("Bad bytes".into()))?;
if HEADER_REGEX.is_match(framing) {
Ok(())
} else {
Expand All @@ -130,8 +127,9 @@ fn check_header(header: &Vec<u8>) -> Result<(), Error> {
}

// Checks footer framing bytes and returns an error if they are invalid
fn check_footer(footer: &Vec<u8>) -> Result<(), Error> {
let framing = str::from_utf8(&footer).unwrap();
fn check_footer(footer: &[u8]) -> Result<(), Error> {
let framing =
str::from_utf8(footer).map_err(|_| ErrorKind::SlatepackDeser("Bad bytes".into()))?;
if FOOTER_REGEX.is_match(framing) {
Ok(())
} else {
Expand Down Expand Up @@ -177,7 +175,7 @@ fn format_slatepack(slatepack: &str) -> Result<String, Error> {
}

// Returns the first four bytes of a double sha256 hash of some bytes
fn generate_check(payload: &Vec<u8>) -> Result<Vec<u8>, Error> {
fn generate_check(payload: &[u8]) -> Result<Vec<u8>, Error> {
let mut first_hash = Sha256::new();
first_hash.input(payload);
let mut second_hash = Sha256::new();
Expand Down
27 changes: 11 additions & 16 deletions libwallet/src/slatepack/packer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@
// limitations under the License.

use std::convert::TryFrom;
use std::iter::FromIterator;
use std::str;

use super::armor::HEADER;
use crate::{Error, ErrorKind};
use crate::{
Slate, SlateVersion, Slatepack, SlatepackAddress, SlatepackArmor, SlatepackBin,
Expand Down Expand Up @@ -47,30 +48,24 @@ impl<'a> Slatepacker<'a> {
}

/// return slatepack
pub fn deser_slatepack(&self, data: Vec<u8>, decrypt: bool) -> Result<Slatepack, Error> {
pub fn deser_slatepack(&self, data: &[u8], decrypt: bool) -> Result<Slatepack, Error> {
// check if data is armored, if so, remove and continue
if data.len() < super::armor::HEADER.len() {
let msg = format!("Data too short");
return Err(ErrorKind::SlatepackDeser(msg).into());
}
let test_header = Vec::from_iter(data[0..super::armor::HEADER.len()].iter().cloned());
let data = match String::from_utf8(test_header) {

let test_header = &data[..HEADER.len()];

let data = match str::from_utf8(test_header) {
Ok(s) => {
if s.as_str() == super::armor::HEADER {
SlatepackArmor::decode(
String::from_utf8(data)
.map_err(|e| {
let msg = format!("{}", e);
error!("Error decoding slatepack armor: {}", msg);
ErrorKind::SlatepackDeser(msg)
})?
.as_str(),
)?
if s == HEADER {
SlatepackArmor::decode(data)?
} else {
data
data.to_vec()
}
}
Err(_) => data,
Err(_) => data.to_vec(),
};

// try as bin first, then as json
Expand Down
4 changes: 2 additions & 2 deletions libwallet/src/slatepack/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ impl Slatepack {
}

/// retrieve recipients
pub fn recipients(&self) -> &Vec<SlatepackAddress> {
pub fn recipients(&self) -> &[SlatepackAddress] {
&self.encrypted_meta.recipients
}

Expand Down Expand Up @@ -453,7 +453,7 @@ pub struct SlatepackEncMetadata {
recipients: Vec<SlatepackAddress>,
}

fn recipients_empty(value: &Vec<SlatepackAddress>) -> bool {
fn recipients_empty(value: &[SlatepackAddress]) -> bool {
value.is_empty()
}

Expand Down
2 changes: 1 addition & 1 deletion src/cmd/wallet_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ fn prompt_slatepack() -> Result<String, ParseError> {
}
}
ReadResult::Input(line) => {
if SlatepackArmor::decode(&line).is_ok() {
if SlatepackArmor::decode(line.as_bytes()).is_ok() {
message = line;
break;
} else {
Expand Down

0 comments on commit e00c61f

Please sign in to comment.