Skip to content

Commit

Permalink
rust: update toolchain to 1.84.0 and update bindgen/cbindgen
Browse files Browse the repository at this point in the history
This reduces the binary size by about 3kB.

New now are warnings (which we interpret as errors) about shared mut
refs:

```
 = note: shared references to mutable statics are dangerous; it's undefined behavior if the static is mutated or if a mutable reference is created for it while the shared reference lives
   = note: `-D static-mut-refs` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(static_mut_refs)]`
error: creating a shared reference to mutable static is discouraged
```

The commit includes `UnsafeSyncRefCell` and uses it in some unit tests
to remove the warning. In `src/rust/bitbox02-rust-c/src/workflow.rs`
we disable the warning altogether as it's hard to fix it there, so we
delay it there for a future PR.
  • Loading branch information
benma committed Jan 13, 2025
1 parent 160e154 commit 7c4bddb
Show file tree
Hide file tree
Showing 1,149 changed files with 402,739 additions and 190,986 deletions.
2 changes: 1 addition & 1 deletion .containerversion
Original file line number Diff line number Diff line change
@@ -1 +1 @@
43
44
6 changes: 3 additions & 3 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -150,15 +150,15 @@ RUN rustup target add thumbv7em-none-eabi
RUN rustup component add rustfmt
RUN rustup component add clippy
RUN rustup component add rust-src
RUN CARGO_HOME=/opt/cargo cargo install cbindgen --version 0.26.0 --locked
RUN CARGO_HOME=/opt/cargo cargo install bindgen-cli --version 0.69.4 --locked
RUN CARGO_HOME=/opt/cargo cargo install cbindgen --version 0.27.0 --locked
RUN CARGO_HOME=/opt/cargo cargo install bindgen-cli --version 0.71.1 --locked

# Until cargo vendor supports vendoring dependencies of the rust std libs we
# need a copy of this file next to the toml file. It also has to be world
# writable so that invocations of `cargo vendor` can update it. Below is the
# tracking issue for `cargo vendor` to support rust std libs.
# https://github.com/rust-lang/wg-cargo-std-aware/issues/23
RUN cp "$(rustc --print=sysroot)/lib/rustlib/src/rust/Cargo.lock" "$(rustc --print=sysroot)/lib/rustlib/src/rust/library/test/"
RUN cp "$(rustc --print=sysroot)/lib/rustlib/src/rust/library/Cargo.lock" "$(rustc --print=sysroot)/lib/rustlib/src/rust/library/test/"
RUN chmod 777 $(rustc --print=sysroot)/lib/rustlib/src/rust/library/test/Cargo.lock

COPY tools/prost-build-proto prost-build-proto
Expand Down
4 changes: 4 additions & 0 deletions src/rust/bitbox02-rust-c/src/workflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@
//! usb message proessing is not ported to Rust. If that happens, the `async_usb` module can be
//! used and this can be deleted.
// TODO: figure out how to deal with the static muts below.
// https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html
#![allow(static_mut_refs)]

extern crate alloc;

use alloc::boxed::Box;
Expand Down
16 changes: 8 additions & 8 deletions src/rust/bitbox02-rust/src/hww.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,23 +185,23 @@ mod tests {
};
if verification_required {
// Verify pairing code.
static mut EXPECTED_PAIRING_CODE: Option<String> = None;
static EXPECTED_PAIRING_CODE: util::UnsafeSyncRefCell<Option<String>> =
util::UnsafeSyncRefCell::new(None);

// Handshake hash as computed by the host. Should be the same as computed on the device. The
// pairing code is derived from that.
let handshake_hash: bitbox02_noise::HandshakeHash =
host_noise.get_hash().try_into().unwrap();
unsafe {
EXPECTED_PAIRING_CODE =
Some(crate::workflow::pairing::format_hash(&handshake_hash));
}
*EXPECTED_PAIRING_CODE.borrow_mut() =
Some(crate::workflow::pairing::format_hash(&handshake_hash));
static mut PAIRING_CONFIRMED: bool = false;
mock(Data {
ui_confirm_create: Some(Box::new(|params| {
assert_eq!(params.title, "Pairing code");
assert_eq!(params.body, unsafe {
EXPECTED_PAIRING_CODE.as_ref().unwrap().as_str()
});
assert_eq!(
params.body,
EXPECTED_PAIRING_CODE.borrow().as_ref().unwrap().as_str()
);
unsafe {
PAIRING_CONFIRMED = true;
}
Expand Down
6 changes: 2 additions & 4 deletions src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,7 @@ struct WalletPolicyPkTranslator<'a> {
address_index: u32,
}

impl<'a> miniscript::Translator<String, bitcoin::PublicKey, Error>
for WalletPolicyPkTranslator<'a>
{
impl miniscript::Translator<String, bitcoin::PublicKey, Error> for WalletPolicyPkTranslator<'_> {
fn pk(&mut self, pk: &String) -> Result<bitcoin::PublicKey, Error> {
let (key_index, multipath_index_left, multipath_index_right) =
parse_wallet_policy_pk(pk).or(Err(Error::InvalidInput))?;
Expand Down Expand Up @@ -273,7 +271,7 @@ pub struct ParsedPolicy<'a> {
pub descriptor: Descriptor<String>,
}

impl<'a> ParsedPolicy<'a> {
impl ParsedPolicy<'_> {
/// Iterates over the placeholder keys in this descriptor. For tr() descriptors, this covers the
/// internal key and every key in every leaf script.
/// This iterates the keys "left-to-right" in the descriptor.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ impl<'a, U: Update> HashedWriter<'a, U> {
}
}

impl<'a, U: Update> Write for HashedWriter<'a, U> {
impl<U: Update> Write for HashedWriter<'_, U> {
type Error = ();
fn write_all(&mut self, buf: &[u8]) -> Result<(), Self::Error> {
self.0.update(buf);
Expand Down
2 changes: 1 addition & 1 deletion src/rust/bitbox02-rust/src/hww/api/ethereum/amount.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub struct Amount<'a> {
pub value: BigUint,
}

impl<'a> Amount<'a> {
impl Amount<'_> {
/// Formats the amount with the right number of decimal places, suffixed with the unit. If the
/// value (without the unit suffix) is too long to fit on the screen, it will be truncated and
/// ellipsis ('...') are appended.
Expand Down
2 changes: 1 addition & 1 deletion src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub enum Transaction<'a> {
Eip1559(&'a pb::EthSignEip1559Request),
}

impl<'a> Transaction<'a> {
impl Transaction<'_> {
fn nonce(&self) -> &[u8] {
match self {
Transaction::Legacy(legacy) => &legacy.nonce,
Expand Down
4 changes: 2 additions & 2 deletions src/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ mod tests {
Struct(Vec<Object<'a>>),
}

impl<'a> Object<'a> {
impl Object<'_> {
fn encode(&self) -> Vec<u8> {
match self {
Object::String(s) => s.as_bytes().to_vec(),
Expand Down Expand Up @@ -680,7 +680,7 @@ mod tests {
message: Object<'a>,
}

impl<'a> TypedMessage<'a> {
impl TypedMessage<'_> {
/// The host is asked for a value at a member of an object. This handles this request and
/// responds with value.
fn handle_host_response(
Expand Down
58 changes: 22 additions & 36 deletions src/rust/bitbox02-rust/src/xpubcache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,24 +127,20 @@ mod tests {
#[derive(Clone)]
struct MockXpub(Vec<u32>);

static mut CHILD_DERIVATIONS: u32 = 0;
static mut ROOT_DERIVATIONS: u32 = 0;
static CHILD_DERIVATIONS: util::UnsafeSyncRefCell<u32> = util::UnsafeSyncRefCell::new(0);
static ROOT_DERIVATIONS: util::UnsafeSyncRefCell<u32> = util::UnsafeSyncRefCell::new(0);

impl Xpub for MockXpub {
fn derive(&self, keypath: &[u32]) -> Result<Self, ()> {
let mut kp = Vec::new();
kp.extend_from_slice(&self.0);
kp.extend_from_slice(keypath);
unsafe {
CHILD_DERIVATIONS += keypath.len() as u32;
}
*CHILD_DERIVATIONS.borrow_mut() += keypath.len() as u32;
Ok(MockXpub(kp))
}

fn from_keypath(keypath: &[u32]) -> Result<Self, ()> {
unsafe {
ROOT_DERIVATIONS += 1;
}
*ROOT_DERIVATIONS.borrow_mut() += 1;
Ok(MockXpub(keypath.to_vec()))
}
}
Expand All @@ -154,18 +150,14 @@ mod tests {
let mut cache = MockCache::new();

assert_eq!(cache.get_xpub(&[]).unwrap().0.as_slice(), &[]);
unsafe {
assert_eq!(CHILD_DERIVATIONS, 0u32);
assert_eq!(ROOT_DERIVATIONS, 1u32);
ROOT_DERIVATIONS = 0;
}
assert_eq!(*CHILD_DERIVATIONS.borrow(), 0u32);
assert_eq!(*ROOT_DERIVATIONS.borrow(), 1u32);
*ROOT_DERIVATIONS.borrow_mut() = 0;

assert_eq!(cache.get_xpub(&[1, 2, 3]).unwrap().0.as_slice(), &[1, 2, 3]);
unsafe {
assert_eq!(CHILD_DERIVATIONS, 0u32);
assert_eq!(ROOT_DERIVATIONS, 1u32);
ROOT_DERIVATIONS = 0;
}
assert_eq!(*CHILD_DERIVATIONS.borrow(), 0u32);
assert_eq!(*ROOT_DERIVATIONS.borrow(), 1u32);
*ROOT_DERIVATIONS.borrow_mut() = 0;

// Cache some keypaths.
cache.add_keypath(&[84 + HARDENED, 0 + HARDENED, 0 + HARDENED]);
Expand All @@ -179,15 +171,13 @@ mod tests {
.as_slice(),
&[84 + HARDENED, 0 + HARDENED, 0 + HARDENED, 1, 2]
);
unsafe {
// Two child derivations:
// 1: m/84'/0'/0' -> m/84'/0'/0'/1
// 2: m/84'/0'/0'/1 -> m/84'/0'/0'/1/2
assert_eq!(CHILD_DERIVATIONS, 2u32);
assert_eq!(ROOT_DERIVATIONS, 1u32);
CHILD_DERIVATIONS = 0;
ROOT_DERIVATIONS = 0;
}
// Two child derivations:
// 1: m/84'/0'/0' -> m/84'/0'/0'/1
// 2: m/84'/0'/0'/1 -> m/84'/0'/0'/1/2
assert_eq!(*CHILD_DERIVATIONS.borrow(), 2u32);
*CHILD_DERIVATIONS.borrow_mut() = 0;
assert_eq!(*ROOT_DERIVATIONS.borrow(), 1u32);
*ROOT_DERIVATIONS.borrow_mut() = 0;

// Same keypath again is a cache hit at m/84'/0'/0'/1 with one child derivation.
assert_eq!(
Expand All @@ -198,11 +188,9 @@ mod tests {
.as_slice(),
&[84 + HARDENED, 0 + HARDENED, 0 + HARDENED, 1, 2]
);
unsafe {
assert_eq!(CHILD_DERIVATIONS, 1u32);
assert_eq!(ROOT_DERIVATIONS, 0u32);
CHILD_DERIVATIONS = 0;
}
assert_eq!(*CHILD_DERIVATIONS.borrow(), 1u32);
*CHILD_DERIVATIONS.borrow_mut() = 0;
assert_eq!(*ROOT_DERIVATIONS.borrow(), 0u32);

// m/84'/0'/0'/0/0 is a cache hit at m/84'/0'/0', which was cached because of the above we
// call using m/84'/0'/0'/1/2.
Expand All @@ -214,10 +202,8 @@ mod tests {
.as_slice(),
&[84 + HARDENED, 0 + HARDENED, 0 + HARDENED, 0, 0]
);
unsafe {
assert_eq!(CHILD_DERIVATIONS, 2u32);
assert_eq!(ROOT_DERIVATIONS, 0u32);
}
assert_eq!(*CHILD_DERIVATIONS.borrow(), 2u32);
assert_eq!(*ROOT_DERIVATIONS.borrow(), 0u32);
}

#[test]
Expand Down
4 changes: 2 additions & 2 deletions src/rust/bitbox02/src/ui/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub struct Component<'a> {
_p: PhantomData<&'a ()>,
}

impl<'a> Component<'a> {
impl Component<'_> {
pub fn screen_stack_push(&mut self) {
if self.is_pushed {
panic!("component pushed twice");
Expand All @@ -48,7 +48,7 @@ impl<'a> Component<'a> {
}
}

impl<'a> Drop for Component<'a> {
impl Drop for Component<'_> {
fn drop(&mut self) {
if !self.is_pushed {
panic!("component not pushed");
Expand Down
4 changes: 2 additions & 2 deletions src/rust/bitbox02/src/ui/ui_stub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub struct Component<'a> {
_p: PhantomData<&'a ()>,
}

impl<'a> Component<'a> {
impl Component<'_> {
pub fn screen_stack_push(&mut self) {
if self.is_pushed {
panic!("component pushed twice");
Expand All @@ -39,7 +39,7 @@ impl<'a> Component<'a> {
}
}

impl<'a> Drop for Component<'a> {
impl Drop for Component<'_> {
fn drop(&mut self) {
if !self.is_pushed {
panic!("component not pushed");
Expand Down
1 change: 1 addition & 0 deletions src/rust/bitbox02/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ mod tests {
}

#[test]
#[allow(clippy::manual_c_str_literals)]
fn test_str_from_null_terminated_ptr() {
assert_eq!(
unsafe { str_from_null_terminated_ptr(b"\0".as_ptr()) },
Expand Down
2 changes: 1 addition & 1 deletion src/rust/rust-toolchain.toml
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
[toolchain]
channel = "1.80.0"
channel = "1.84.0"
21 changes: 20 additions & 1 deletion src/rust/util/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub struct Survive<'a, T: 'a> {
phantom: core::marker::PhantomData<&'a T>,
}

impl<'a, T> Survive<'a, T> {
impl<T> Survive<'_, T> {
pub fn new(data: T) -> Self {
Survive {
data,
Expand All @@ -50,6 +50,25 @@ impl<'a, T> Survive<'a, T> {
}
}

/// A wrapper that adds the Sync trait to RefCell. We can use this to avoid mutexes as our target is
/// single-threaded and our unit tests run single-threaded.
pub struct UnsafeSyncRefCell<T>(pub core::cell::RefCell<T>);
impl<T> UnsafeSyncRefCell<T> {
pub const fn new(value: T) -> Self {
Self(core::cell::RefCell::new(value))
}
}

unsafe impl<T> Sync for UnsafeSyncRefCell<T> {}

impl<T> core::ops::Deref for UnsafeSyncRefCell<T> {
type Target = core::cell::RefCell<T>;

fn deref(&self) -> &Self::Target {
&self.0
}
}

#[cfg(test)]
mod tests {
extern crate std;
Expand Down
1 change: 1 addition & 0 deletions src/rust/vendor/addr2line/.cargo-checksum.json

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

Loading

0 comments on commit 7c4bddb

Please sign in to comment.