Skip to content

Speed up deserialization of secp256k1 objects significantly #933

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

Merged
merged 6 commits into from
Jun 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,8 @@ panic = "abort"
opt-level = 3
lto = true
panic = "abort"

[profile.bench]
opt-level = 3
codegen-units = 1
lto = true
6 changes: 3 additions & 3 deletions lightning/src/ln/onion_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use ln::{PaymentHash, PaymentSecret};
use ln::channelmanager::HTLCSource;
use ln::msgs;
use routing::router::RouteHop;
use util::byte_utils;
use util::chacha20::ChaCha20;
use util::errors::{self, APIError};
use util::ser::{Readable, Writeable, LengthCalculatingWriter};
Expand All @@ -29,6 +28,7 @@ use bitcoin::secp256k1;

use prelude::*;
use std::io::Cursor;
use core::convert::TryInto;
use core::ops::Deref;

pub(super) struct OnionKeys {
Expand Down Expand Up @@ -367,7 +367,7 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
const NODE: u16 = 0x2000;
const UPDATE: u16 = 0x1000;

let error_code = byte_utils::slice_to_be16(&error_code_slice);
let error_code = u16::from_be_bytes(error_code_slice.try_into().expect("len is 2"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the message will prefix the error, so maybe something specific to the call site instead for these.

Copy link
Collaborator Author

@TheBlueMatt TheBlueMatt May 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sure hope we never see it anywhere, so I'll just leave it and pretend its a comment.

error_code_ret = Some(error_code);
error_packet_ret = Some(err_packet.failuremsg[2..].to_vec());

Expand All @@ -394,7 +394,7 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
}
else if error_code & UPDATE == UPDATE {
if let Some(update_len_slice) = err_packet.failuremsg.get(debug_field_size+2..debug_field_size+4) {
let update_len = byte_utils::slice_to_be16(&update_len_slice) as usize;
let update_len = u16::from_be_bytes(update_len_slice.try_into().expect("len is 2")) as usize;
if let Some(update_slice) = err_packet.failuremsg.get(debug_field_size + 4..debug_field_size + 4 + update_len) {
if let Ok(chan_update) = msgs::ChannelUpdate::read(&mut Cursor::new(&update_slice)) {
// if channel_update should NOT have caused the failure:
Expand Down
9 changes: 4 additions & 5 deletions lightning/src/ln/peer_channel_encryptor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ use bitcoin::secp256k1::ecdh::SharedSecret;
use bitcoin::secp256k1;

use util::chacha20poly1305rfc::ChaCha20Poly1305RFC;
use util::byte_utils;
use bitcoin::hashes::hex::ToHex;

/// Maximum Lightning message data length according to
Expand Down Expand Up @@ -141,7 +140,7 @@ impl PeerChannelEncryptor {
#[inline]
fn encrypt_with_ad(res: &mut[u8], n: u64, key: &[u8; 32], h: &[u8], plaintext: &[u8]) {
let mut nonce = [0; 12];
nonce[4..].copy_from_slice(&byte_utils::le64_to_array(n));
nonce[4..].copy_from_slice(&n.to_le_bytes()[..]);

let mut chacha = ChaCha20Poly1305RFC::new(key, &nonce, h);
let mut tag = [0; 16];
Expand All @@ -152,7 +151,7 @@ impl PeerChannelEncryptor {
#[inline]
fn decrypt_with_ad(res: &mut[u8], n: u64, key: &[u8; 32], h: &[u8], cyphertext: &[u8]) -> Result<(), LightningError> {
let mut nonce = [0; 12];
nonce[4..].copy_from_slice(&byte_utils::le64_to_array(n));
nonce[4..].copy_from_slice(&n.to_le_bytes()[..]);

let mut chacha = ChaCha20Poly1305RFC::new(key, &nonce, h);
if !chacha.decrypt(&cyphertext[0..cyphertext.len() - 16], res, &cyphertext[cyphertext.len() - 16..]) {
Expand Down Expand Up @@ -406,7 +405,7 @@ impl PeerChannelEncryptor {
*sn = 0;
}

Self::encrypt_with_ad(&mut res[0..16+2], *sn, sk, &[0; 0], &byte_utils::be16_to_array(msg.len() as u16));
Self::encrypt_with_ad(&mut res[0..16+2], *sn, sk, &[0; 0], &(msg.len() as u16).to_be_bytes());
*sn += 1;

Self::encrypt_with_ad(&mut res[16+2..], *sn, sk, &[0; 0], msg);
Expand Down Expand Up @@ -435,7 +434,7 @@ impl PeerChannelEncryptor {
let mut res = [0; 2];
Self::decrypt_with_ad(&mut res, *rn, rk, &[0; 0], msg)?;
*rn += 1;
Ok(byte_utils::slice_to_be16(&res))
Ok(u16::from_be_bytes(res))
},
_ => panic!("Tried to decrypt a message prior to noise handshake completion"),
}
Expand Down
6 changes: 3 additions & 3 deletions lightning/src/ln/wire.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,8 +350,8 @@ impl Encode for msgs::GossipTimestampFilter {
#[cfg(test)]
mod tests {
use super::*;
use util::byte_utils;
use prelude::*;
use core::convert::TryInto;

// Big-endian wire encoding of Pong message (type = 19, byteslen = 2).
const ENCODED_PONG: [u8; 6] = [0u8, 19u8, 0u8, 2u8, 0u8, 0u8];
Expand Down Expand Up @@ -397,7 +397,7 @@ mod tests {

#[test]
fn read_unknown_message() {
let buffer = &byte_utils::be16_to_array(::core::u16::MAX);
let buffer = &::core::u16::MAX.to_be_bytes();
let mut reader = ::std::io::Cursor::new(buffer);
let message = read(&mut reader).unwrap();
match message {
Expand All @@ -414,7 +414,7 @@ mod tests {

let type_length = ::core::mem::size_of::<u16>();
let (type_bytes, payload_bytes) = buffer.split_at(type_length);
assert_eq!(byte_utils::slice_to_be16(type_bytes), msgs::Pong::TYPE);
assert_eq!(u16::from_be_bytes(type_bytes.try_into().unwrap()), msgs::Pong::TYPE);
assert_eq!(payload_bytes, &ENCODED_PONG[type_length..]);
}

Expand Down
27 changes: 27 additions & 0 deletions lightning/src/routing/network_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2424,3 +2424,30 @@ mod tests {
assert!(result.is_err());
}
}

#[cfg(all(test, feature = "unstable"))]
mod benches {
use super::*;

use test::Bencher;
use std::io::Read;

#[bench]
fn read_network_graph(bench: &mut Bencher) {
let mut d = ::routing::router::test_utils::get_route_file().unwrap();
let mut v = Vec::new();
d.read_to_end(&mut v).unwrap();
bench.iter(|| {
let _ = NetworkGraph::read(&mut std::io::Cursor::new(&v)).unwrap();
});
}

#[bench]
fn write_network_graph(bench: &mut Bencher) {
let mut d = ::routing::router::test_utils::get_route_file().unwrap();
let net_graph = NetworkGraph::read(&mut d).unwrap();
bench.iter(|| {
let _ = net_graph.encode();
});
}
}
71 changes: 36 additions & 35 deletions lightning/src/routing/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3856,44 +3856,21 @@ mod tests {
}
}

use std::fs::File;
use util::ser::Readable;
/// Tries to open a network graph file, or panics with a URL to fetch it.
pub(super) fn get_route_file() -> Result<std::fs::File, std::io::Error> {
let res = File::open("net_graph-2021-05-27.bin") // By default we're run in RL/lightning
.or_else(|_| File::open("lightning/net_graph-2021-05-27.bin")) // We may be run manually in RL/
.or_else(|_| { // Fall back to guessing based on the binary location
// path is likely something like .../rust-lightning/target/debug/deps/lightning-...
let mut path = std::env::current_exe().unwrap();
path.pop(); // lightning-...
path.pop(); // deps
path.pop(); // debug
path.pop(); // target
path.push("lightning");
path.push("net_graph-2021-05-27.bin");
eprintln!("{}", path.to_str().unwrap());
File::open(path)
});
#[cfg(require_route_graph_test)]
return Ok(res.expect("Didn't have route graph and was configured to require it"));
#[cfg(not(require_route_graph_test))]
return res;
}

pub(super) fn random_init_seed() -> u64 {
// Because the default HashMap in std pulls OS randomness, we can use it as a (bad) RNG.
use core::hash::{BuildHasher, Hasher};
let seed = std::collections::hash_map::RandomState::new().build_hasher().finish();
println!("Using seed of {}", seed);
seed
}
use util::ser::Readable;

#[test]
fn generate_routes() {
let mut d = match get_route_file() {
let mut d = match super::test_utils::get_route_file() {
Ok(f) => f,
Err(_) => {
eprintln!("Please fetch https://bitcoin.ninja/ldk-net_graph-45d86ead641d-2021-05-27.bin and place it at lightning/net_graph-2021-05-27.bin");
Err(e) => {
eprintln!("{}", e);
return;
},
};
Expand All @@ -3917,10 +3894,10 @@ mod tests {

#[test]
fn generate_routes_mpp() {
let mut d = match get_route_file() {
let mut d = match super::test_utils::get_route_file() {
Ok(f) => f,
Err(_) => {
eprintln!("Please fetch https://bitcoin.ninja/ldk-net_graph-45d86ead641d-2021-05-27.bin and place it at lightning/net_graph-2021-05-27.bin");
Err(e) => {
eprintln!("{}", e);
return;
},
};
Expand All @@ -3943,12 +3920,38 @@ mod tests {
}
}

#[cfg(test)]
pub(crate) mod test_utils {
use std::fs::File;
/// Tries to open a network graph file, or panics with a URL to fetch it.
pub(crate) fn get_route_file() -> Result<std::fs::File, &'static str> {
let res = File::open("net_graph-2021-05-27.bin") // By default we're run in RL/lightning
.or_else(|_| File::open("lightning/net_graph-2021-05-27.bin")) // We may be run manually in RL/
.or_else(|_| { // Fall back to guessing based on the binary location
// path is likely something like .../rust-lightning/target/debug/deps/lightning-...
let mut path = std::env::current_exe().unwrap();
path.pop(); // lightning-...
path.pop(); // deps
path.pop(); // debug
path.pop(); // target
path.push("lightning");
path.push("net_graph-2021-05-27.bin");
eprintln!("{}", path.to_str().unwrap());
File::open(path)
})
.map_err(|_| "Please fetch https://bitcoin.ninja/ldk-net_graph-45d86ead641d-2021-05-27.bin and place it at lightning/net_graph-2021-05-27.bin");
#[cfg(require_route_graph_test)]
return Ok(res.unwrap());
#[cfg(not(require_route_graph_test))]
return res;
}
}

#[cfg(all(test, feature = "unstable"))]
mod benches {
use super::*;
use util::logger::{Logger, Record};

use prelude::*;
use test::Bencher;

struct DummyLogger {}
Expand All @@ -3958,8 +3961,7 @@ mod benches {

#[bench]
fn generate_routes(bench: &mut Bencher) {
let mut d = tests::get_route_file()
.expect("Please fetch https://bitcoin.ninja/ldk-net_graph-45d86ead641d-2021-05-27.bin and place it at lightning/net_graph-2021-05-27.bin");
let mut d = test_utils::get_route_file().unwrap();
let graph = NetworkGraph::read(&mut d).unwrap();

// First, get 100 (source, destination) pairs for which route-getting actually succeeds...
Expand Down Expand Up @@ -3990,8 +3992,7 @@ mod benches {

#[bench]
fn generate_mpp_routes(bench: &mut Bencher) {
let mut d = tests::get_route_file()
.expect("Please fetch https://bitcoin.ninja/ldk-net_graph-45d86ead641d-2021-05-27.bin and place it at lightning/net_graph-2021-05-27.bin");
let mut d = test_utils::get_route_file().unwrap();
let graph = NetworkGraph::read(&mut d).unwrap();

// First, get 100 (source, destination) pairs for which route-getting actually succeeds...
Expand Down
34 changes: 0 additions & 34 deletions lightning/src/util/byte_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,6 @@
// You may not use this file except in accordance with one or both of these
// licenses.

#[inline]
pub fn slice_to_be16(v: &[u8]) -> u16 {
((v[0] as u16) << 8*1) |
((v[1] as u16) << 8*0)
}
#[inline]
pub fn slice_to_be32(v: &[u8]) -> u32 {
((v[0] as u32) << 8*3) |
((v[1] as u32) << 8*2) |
((v[2] as u32) << 8*1) |
((v[3] as u32) << 8*0)
}
#[cfg(not(feature = "fuzztarget"))] // Used only by poly1305
#[inline]
pub fn slice_to_le32(v: &[u8]) -> u32 {
((v[0] as u32) << 8*0) |
((v[1] as u32) << 8*1) |
((v[2] as u32) << 8*2) |
((v[3] as u32) << 8*3)
}
#[inline]
pub fn slice_to_be48(v: &[u8]) -> u64 {
((v[0] as u64) << 8*5) |
Expand Down Expand Up @@ -64,16 +44,6 @@ pub fn be32_to_array(u: u32) -> [u8; 4] {
v[3] = ((u >> 8*0) & 0xff) as u8;
v
}
#[cfg(not(feature = "fuzztarget"))] // Used only by poly1305
#[inline]
pub fn le32_to_array(u: u32) -> [u8; 4] {
let mut v = [0; 4];
v[0] = ((u >> 8*0) & 0xff) as u8;
v[1] = ((u >> 8*1) & 0xff) as u8;
v[2] = ((u >> 8*2) & 0xff) as u8;
v[3] = ((u >> 8*3) & 0xff) as u8;
v
}
#[inline]
pub fn be48_to_array(u: u64) -> [u8; 6] {
assert!(u & 0xffff_0000_0000_0000 == 0);
Expand Down Expand Up @@ -120,14 +90,10 @@ mod tests {

#[test]
fn test_all() {
assert_eq!(slice_to_be16(&[0xde, 0xad]), 0xdead);
assert_eq!(slice_to_be32(&[0xde, 0xad, 0xbe, 0xef]), 0xdeadbeef);
assert_eq!(slice_to_le32(&[0xef, 0xbe, 0xad, 0xde]), 0xdeadbeef);
assert_eq!(slice_to_be48(&[0xde, 0xad, 0xbe, 0xef, 0x1b, 0xad]), 0xdeadbeef1bad);
assert_eq!(slice_to_be64(&[0xde, 0xad, 0xbe, 0xef, 0x1b, 0xad, 0x1d, 0xea]), 0xdeadbeef1bad1dea);
assert_eq!(be16_to_array(0xdead), [0xde, 0xad]);
assert_eq!(be32_to_array(0xdeadbeef), [0xde, 0xad, 0xbe, 0xef]);
assert_eq!(le32_to_array(0xdeadbeef), [0xef, 0xbe, 0xad, 0xde]);
assert_eq!(be48_to_array(0xdeadbeef1bad), [0xde, 0xad, 0xbe, 0xef, 0x1b, 0xad]);
assert_eq!(be64_to_array(0xdeadbeef1bad1dea), [0xde, 0xad, 0xbe, 0xef, 0x1b, 0xad, 0x1d, 0xea]);
assert_eq!(le64_to_array(0xdeadbeef1bad1dea), [0xea, 0x1d, 0xad, 0x1b, 0xef, 0xbe, 0xad, 0xde]);
Expand Down
Loading