Skip to content
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

Use debug builds in our Dockerfiles to reduce CI times #462

Merged
merged 4 commits into from
Nov 29, 2023
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
60 changes: 38 additions & 22 deletions coins/monero/src/wallet/send/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ pub struct SignableTransaction {
protocol: Protocol,
r_seed: Option<Zeroizing<[u8; 32]>>,
inputs: Vec<(SpendableOutput, Decoys)>,
has_change: bool,
payments: Vec<InternalPayment>,
data: Vec<Vec<u8>>,
fee: u64,
Expand Down Expand Up @@ -385,7 +386,7 @@ fn need_additional(payments: &[InternalPayment]) -> (bool, bool) {
(subaddresses, additional)
}

fn sanity_check_change_payment(payments: &[InternalPayment], has_change_address: bool) {
fn sanity_check_change_payment_quantity(payments: &[InternalPayment], has_change_address: bool) {
debug_assert_eq!(
payments
.iter()
Expand Down Expand Up @@ -516,7 +517,7 @@ impl SignableTransaction {
}

// Sanity check we have the expected number of change outputs
sanity_check_change_payment(&payments, change_address.is_some());
sanity_check_change_payment_quantity(&payments, change_address.is_some());

// Modify the amount of the change output
if change_address.is_some() {
Expand All @@ -527,23 +528,34 @@ impl SignableTransaction {
}

// Sanity check the change again after modifying
sanity_check_change_payment(&payments, change_address.is_some());
sanity_check_change_payment_quantity(&payments, change_address.is_some());

// Sanity check outgoing amount + fee == incoming amount
debug_assert_eq!(
payments
.iter()
.map(|payment| match *payment {
InternalPayment::Payment(payment) => payment.1,
InternalPayment::Change(_, amount) => amount,
})
.sum::<u64>() +
fee,
in_amount,
"Outgoing amount + fee != incoming amount"
);
if change_address.is_some() {
debug_assert_eq!(
payments
.iter()
.map(|payment| match *payment {
InternalPayment::Payment(payment) => payment.1,
InternalPayment::Change(_, amount) => amount,
})
.sum::<u64>() +
fee,
in_amount,
"Outgoing amount + fee != incoming amount"
);
}

Ok(SignableTransaction { protocol, r_seed, inputs, payments, data, fee, fee_rate })
Ok(SignableTransaction {
protocol,
r_seed,
inputs,
payments,
has_change: change_address.is_some(),
data,
fee,
fee_rate,
})
}

pub fn fee(&self) -> u64 {
Expand Down Expand Up @@ -778,7 +790,9 @@ impl SignableTransaction {
});
encrypted_amounts.push(EncryptedAmount::Compact { amount: output.amount });
}
debug_assert_eq!(self.fee, fee, "transaction will use an unexpected fee");
if self.has_change {
debug_assert_eq!(self.fee, fee, "transaction will use an unexpected fee");
}

(
Transaction {
Expand Down Expand Up @@ -844,11 +858,13 @@ impl SignableTransaction {
_ => unreachable!("attempted to sign a TX which wasn't CLSAG"),
}

debug_assert_eq!(
self.fee_rate.calculate_fee_from_weight(tx.weight()),
tx.rct_signatures.base.fee,
"transaction used unexpected fee",
);
if self.has_change {
debug_assert_eq!(
self.fee_rate.calculate_fee_from_weight(tx.weight()),
tx.rct_signatures.base.fee,
"transaction used unexpected fee",
);
}

Ok(tx)
}
Expand Down
9 changes: 4 additions & 5 deletions coordinator/src/p2p.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ pub trait P2p: Send + Sync + Clone + fmt::Debug + TributaryP2p {
#[derive(NetworkBehaviour)]
struct Behavior {
gossipsub: GsBehavior,
//#[cfg(debug_assertions)]
#[cfg(debug_assertions)]
mdns: libp2p::mdns::tokio::Behaviour,
}

Expand Down Expand Up @@ -261,8 +261,7 @@ impl LibP2p {
},

// Only use MDNS in debug environments, as it should have no value in a release build
// TODO: We do tests on release binaries as of right now...
//#[cfg(debug_assertions)]
#[cfg(debug_assertions)]
mdns: {
log::info!("creating mdns service");
libp2p::mdns::tokio::Behaviour::new(libp2p::mdns::Config::default(), throwaway_peer_id)
Expand Down Expand Up @@ -371,7 +370,7 @@ impl LibP2p {
// Handle new incoming messages
event = swarm.next() => {
match event {
//#[cfg(debug_assertions)]
#[cfg(debug_assertions)]
Some(SwarmEvent::Behaviour(BehaviorEvent::Mdns(
libp2p::mdns::Event::Discovered(list),
))) => {
Expand All @@ -383,7 +382,7 @@ impl LibP2p {
}
}
}
//#[cfg(debug_assertions)]
#[cfg(debug_assertions)]
Some(SwarmEvent::Behaviour(BehaviorEvent::Mdns(
libp2p::mdns::Event::Expired(list),
))) => {
Expand Down
4 changes: 2 additions & 2 deletions orchestration/coordinator/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ RUN --mount=type=cache,target=/root/.cargo \
--mount=type=cache,target=/usr/local/cargo/git \
--mount=type=cache,target=/serai/target \
mkdir /serai/bin && \
cargo build -p serai-coordinator --release --all-features && \
mv /serai/target/release/serai-coordinator /serai/bin
cargo build -p serai-coordinator --all-features && \
mv /serai/target/debug/serai-coordinator /serai/bin
FROM debian:bookworm-slim as image

COPY --from=mimalloc libmimalloc.so /usr/lib
Expand Down
4 changes: 2 additions & 2 deletions orchestration/coordinator/Dockerfile.coordinator
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
cargo build -p serai-coordinator --release --all-features && \
mv /serai/target/release/serai-coordinator /serai/bin
cargo build -p serai-coordinator --all-features && \
mv /serai/target/debug/serai-coordinator /serai/bin
4 changes: 2 additions & 2 deletions orchestration/message-queue/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ RUN --mount=type=cache,target=/root/.cargo \
--mount=type=cache,target=/usr/local/cargo/git \
--mount=type=cache,target=/serai/target \
mkdir /serai/bin && \
cargo build --release --all-features -p serai-message-queue && \
mv /serai/target/release/serai-message-queue /serai/bin
cargo build --all-features -p serai-message-queue && \
mv /serai/target/debug/serai-message-queue /serai/bin
FROM debian:bookworm-slim as image

COPY --from=mimalloc libmimalloc.so /usr/lib
Expand Down
4 changes: 2 additions & 2 deletions orchestration/message-queue/Dockerfile.message-queue
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
cargo build --release --all-features -p serai-message-queue && \
mv /serai/target/release/serai-message-queue /serai/bin
cargo build --all-features -p serai-message-queue && \
mv /serai/target/debug/serai-message-queue /serai/bin
4 changes: 2 additions & 2 deletions orchestration/processor/bitcoin/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ RUN --mount=type=cache,target=/root/.cargo \
--mount=type=cache,target=/usr/local/cargo/git \
--mount=type=cache,target=/serai/target \
mkdir /serai/bin && \
cargo build --release --features "binaries bitcoin" -p serai-processor && \
mv /serai/target/release/serai-processor /serai/bin
cargo build --features "binaries bitcoin" -p serai-processor && \
mv /serai/target/debug/serai-processor /serai/bin
FROM debian:bookworm-slim as image

COPY --from=mimalloc libmimalloc.so /usr/lib
Expand Down
4 changes: 2 additions & 2 deletions orchestration/processor/bitcoin/Dockerfile.processor.bitcoin
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
cargo build --release --features "binaries bitcoin" -p serai-processor && \
mv /serai/target/release/serai-processor /serai/bin
cargo build --features "binaries bitcoin" -p serai-processor && \
mv /serai/target/debug/serai-processor /serai/bin
4 changes: 2 additions & 2 deletions orchestration/processor/monero/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ RUN --mount=type=cache,target=/root/.cargo \
--mount=type=cache,target=/usr/local/cargo/git \
--mount=type=cache,target=/serai/target \
mkdir /serai/bin && \
cargo build --release --features "binaries monero" -p serai-processor && \
mv /serai/target/release/serai-processor /serai/bin
cargo build --features "binaries monero" -p serai-processor && \
mv /serai/target/debug/serai-processor /serai/bin
FROM debian:bookworm-slim as image

COPY --from=mimalloc libmimalloc.so /usr/lib
Expand Down
4 changes: 2 additions & 2 deletions orchestration/processor/monero/Dockerfile.processor.monero
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
cargo build --release --features "binaries monero" -p serai-processor && \
mv /serai/target/release/serai-processor /serai/bin
cargo build --features "binaries monero" -p serai-processor && \
mv /serai/target/debug/serai-processor /serai/bin
7 changes: 4 additions & 3 deletions processor/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,12 +569,13 @@ async fn run<N: Network, D: Db, Co: Coordinator>(mut raw_db: D, network: N, mut
// KeyGen specifically may take a notable amount of processing time
// While that shouldn't be an issue in practice, as after processing an attempt it'll handle
// the other messages in the queue, it may be beneficial to parallelize these
// They could likely be parallelized by type (KeyGen, Sign, Substrate) without issue
// They could potentially be parallelized by type (KeyGen, Sign, Substrate) without issue
msg = coordinator.recv() => {
assert_eq!(msg.id, (last_coordinator_msg.unwrap_or(msg.id - 1) + 1));
if let Some(last_coordinator_msg) = last_coordinator_msg {
assert_eq!(msg.id, last_coordinator_msg + 1);
}
last_coordinator_msg = Some(msg.id);


// Only handle this if we haven't already
if HandledMessageDb::get(&main_db, msg.id).is_none() {
HandledMessageDb::set(&mut txn, msg.id, &());
Expand Down