Skip to content

Commit 27445cc

Browse files
sebastiencssebastiencs
authored
Merge pull request #1007 from openmina/fix-panic
Don't make `OneOrTwo::zip` panic + set `RUST_BACKTRACE`
2 parents 87c3fd3 + 3c97aab commit 27445cc

File tree

5 files changed

+157
-71
lines changed

5 files changed

+157
-71
lines changed

‎cli/src/main.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,20 @@ mod unsafe_signal_handlers {
4545
}
4646
}
4747

48+
fn setup_var_from_single_and_only_thread() {
49+
const VARS: &[(&str, &str)] = &[("RUST_BACKTRACE", "full")];
50+
51+
for (name, value) in VARS {
52+
if std::env::var(name).is_err() {
53+
// Safe to call, we didn't launch any threads yet
54+
unsafe { std::env::set_var(name, value) };
55+
}
56+
}
57+
}
58+
4859
fn main() -> anyhow::Result<()> {
60+
setup_var_from_single_and_only_thread();
61+
4962
#[cfg(feature = "unsafe-signal-handlers")]
5063
unsafe_signal_handlers::setup();
5164
let app = commands::OpenminaCli::parse();

‎core/src/log.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,34 +43,49 @@ macro_rules! trace {
4343
($time:expr; $($tts:tt)*) => {
4444
$crate::log_entry!(trace, $time; $($tts)*);
4545
};
46+
($($tts:tt)*) => {
47+
$crate::log_entry!(trace; $($tts)*);
48+
};
4649
}
4750

4851
#[macro_export]
4952
macro_rules! debug {
5053
($time:expr; $($tts:tt)*) => {
5154
$crate::log_entry!(debug, $time; $($tts)*);
5255
};
56+
($($tts:tt)*) => {
57+
$crate::log_entry!(debug; $($tts)*);
58+
};
5359
}
5460

5561
#[macro_export]
5662
macro_rules! info {
5763
($time:expr; $($tts:tt)*) => {
5864
$crate::log_entry!(info, $time; $($tts)*);
5965
};
66+
($($tts:tt)*) => {
67+
$crate::log_entry!(info; $($tts)*);
68+
};
6069
}
6170

6271
#[macro_export]
6372
macro_rules! warn {
6473
($time:expr; $($tts:tt)*) => {
6574
$crate::log_entry!(warn, $time; $($tts)*);
6675
};
76+
($($tts:tt)*) => {
77+
$crate::log_entry!(warn; $($tts)*);
78+
};
6779
}
6880

6981
#[macro_export]
7082
macro_rules! error {
7183
($time:expr; $($tts:tt)*) => {
7284
$crate::log_entry!(error, $time; $($tts)*);
7385
};
86+
($($tts:tt)*) => {
87+
$crate::log_entry!(error; $($tts)*);
88+
};
7489
}
7590

7691
pub const ACTION_TRACE_TARGET: &str = "openmina_core::log::action";

‎ledger/src/proofs/verification.rs

Lines changed: 115 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -741,15 +741,25 @@ pub fn verify_block(
741741
};
742742

743743
let Ok(protocol_state) = ProtocolState::try_from(protocol_state) else {
744+
openmina_core::warn!(
745+
message = format!("verify_block: Protocol state contains invalid field")
746+
);
744747
return false; // invalid bigint
745748
};
746749
let protocol_state_hash = MinaHash::hash(&protocol_state);
747750

748751
let accum_check =
749752
accumulator_check::accumulator_check(srs, &[protocol_state_proof]).unwrap_or(false);
750753
let verified = verify_impl(&protocol_state_hash, protocol_state_proof, &vk).unwrap_or(false);
754+
let ok = accum_check && verified;
755+
756+
openmina_core::info!(message = format!("verify_block OK={ok:?}"));
757+
758+
if !ok {
759+
on_fail::dump_block_verification(header);
760+
}
751761

752-
accum_check && verified
762+
ok
753763
}
754764

755765
pub fn verify_transaction<'a>(
@@ -781,9 +791,16 @@ pub fn verify_transaction<'a>(
781791

782792
let accum_check =
783793
accumulator_check::accumulator_check(srs, &accum_check_proofs).unwrap_or(false);
784-
785794
let verified = batch_verify_impl(inputs.as_slice()).unwrap_or(false);
786-
accum_check && verified
795+
let ok = accum_check && verified;
796+
797+
openmina_core::info!(message = format!("verify_transactions OK={ok:?}"));
798+
799+
if !ok {
800+
on_fail::dump_tx_verification(&inputs);
801+
}
802+
803+
ok
787804
}
788805

789806
/// https://github.com/MinaProtocol/mina/blob/bfd1009abdbee78979ff0343cc73a3480e862f58/src/lib/crypto/kimchi_bindings/stubs/src/pasta_fq_plonk_proof.rs#L116
@@ -807,18 +824,10 @@ pub fn verify_zkapp(
807824

808825
let ok = accum_check && verified;
809826

810-
openmina_core::info!(openmina_core::log::system_time(); message = format!("verify_zkapp OK={ok:?}"));
827+
openmina_core::info!(message = format!("verify_zkapp OK={ok:?}"));
811828

812-
#[cfg(not(test))]
813829
if !ok {
814-
if let Err(e) = dump_zkapp_verification(verification_key, zkapp_statement, sideloaded_proof)
815-
{
816-
openmina_core::error!(
817-
openmina_core::log::system_time();
818-
message = "Failed to dump zkapp verification",
819-
error = format!("{e:?}")
820-
);
821-
}
830+
on_fail::dump_zkapp_verification(verification_key, zkapp_statement, sideloaded_proof);
822831
}
823832

824833
ok
@@ -916,59 +925,109 @@ where
916925
}
917926

918927
/// Dump data when it fails, to reproduce and compare in OCaml
919-
fn dump_zkapp_verification(
920-
verification_key: &VerificationKey,
921-
zkapp_statement: &ZkappStatement,
922-
sideloaded_proof: &PicklesProofProofsVerified2ReprStableV2,
923-
) -> std::io::Result<()> {
924-
use mina_p2p_messages::binprot;
925-
use mina_p2p_messages::binprot::macros::{BinProtRead, BinProtWrite};
926-
927-
#[derive(Clone, Debug, PartialEq, BinProtRead, BinProtWrite)]
928-
struct VerifyZkapp {
929-
vk: v2::MinaBaseVerificationKeyWireStableV1,
930-
zkapp_statement: v2::MinaBaseZkappStatementStableV2,
931-
proof: v2::PicklesProofProofsVerified2ReprStableV2,
928+
mod on_fail {
929+
use super::*;
930+
931+
pub(super) fn dump_zkapp_verification(
932+
verification_key: &VerificationKey,
933+
zkapp_statement: &ZkappStatement,
934+
sideloaded_proof: &PicklesProofProofsVerified2ReprStableV2,
935+
) {
936+
use mina_p2p_messages::binprot;
937+
use mina_p2p_messages::binprot::macros::{BinProtRead, BinProtWrite};
938+
939+
#[derive(Clone, Debug, PartialEq, BinProtRead, BinProtWrite)]
940+
struct VerifyZkapp {
941+
vk: v2::MinaBaseVerificationKeyWireStableV1,
942+
zkapp_statement: v2::MinaBaseZkappStatementStableV2,
943+
proof: v2::PicklesProofProofsVerified2ReprStableV2,
944+
}
945+
946+
let data = VerifyZkapp {
947+
vk: verification_key.into(),
948+
zkapp_statement: zkapp_statement.into(),
949+
proof: sideloaded_proof.clone(),
950+
};
951+
952+
dump_to_file(&data, "verify_zkapp")
932953
}
933954

934-
let data = VerifyZkapp {
935-
vk: verification_key.into(),
936-
zkapp_statement: zkapp_statement.into(),
937-
proof: sideloaded_proof.clone(),
938-
};
955+
pub(super) fn dump_block_verification(header: &MinaBlockHeaderStableV2) {
956+
dump_to_file(header, "verify_block")
957+
}
939958

940-
let bin = {
941-
let mut vec = Vec::with_capacity(128 * 1024);
942-
data.binprot_write(&mut vec)?;
943-
vec
944-
};
959+
pub(super) fn dump_tx_verification(
960+
txs: &[(
961+
&Statement<SokDigest>,
962+
&PicklesProofProofsVerified2ReprStableV2,
963+
&VK,
964+
)],
965+
) {
966+
let data = txs
967+
.iter()
968+
.map(|(statement, proof, _vk)| {
969+
let statement: v2::MinaStateSnarkedLedgerStateWithSokStableV2 = (*statement).into();
970+
(statement, (*proof).clone())
971+
})
972+
.collect::<Vec<_>>();
945973

946-
let debug_dir = openmina_core::get_debug_dir();
947-
let filename = debug_dir
948-
.join(generate_new_filename("verify_zapp", "binprot", &bin)?)
949-
.to_string_lossy()
950-
.to_string();
951-
std::fs::create_dir_all(&debug_dir)?;
974+
dump_to_file(&data, "verify_txs")
975+
}
952976

953-
let mut file = std::fs::File::create(filename)?;
954-
file.write_all(&bin)?;
955-
file.sync_all()?;
977+
#[allow(unreachable_code)]
978+
fn dump_to_file<D: BinProtWrite>(data: &D, filename: &str) {
979+
#[cfg(test)]
980+
{
981+
let (_, _) = (data, filename); // avoid unused vars
982+
return;
983+
}
956984

957-
Ok(())
958-
}
985+
if let Err(e) = dump_to_file_impl(data, filename) {
986+
openmina_core::error!(
987+
message = "Failed to dump proof verification data",
988+
error = format!("{e:?}")
989+
);
990+
}
991+
}
992+
993+
fn dump_to_file_impl<D: BinProtWrite>(data: &D, filename: &str) -> std::io::Result<()> {
994+
let bin = {
995+
let mut vec = Vec::with_capacity(128 * 1024);
996+
data.binprot_write(&mut vec)?;
997+
vec
998+
};
999+
1000+
let debug_dir = openmina_core::get_debug_dir();
1001+
let filename = debug_dir
1002+
.join(generate_new_filename(filename, "binprot", &bin)?)
1003+
.to_string_lossy()
1004+
.to_string();
1005+
std::fs::create_dir_all(&debug_dir)?;
1006+
1007+
let mut file = std::fs::File::create(&filename)?;
1008+
file.write_all(&bin)?;
1009+
file.sync_all()?;
1010+
1011+
openmina_core::error!(
1012+
message = format!("proof verication failed, dumped data to {:?}", &filename)
1013+
);
1014+
1015+
Ok(())
1016+
}
9591017

960-
fn generate_new_filename(name: &str, extension: &str, data: &[u8]) -> std::io::Result<String> {
961-
use crate::proofs::util::sha256_sum;
1018+
fn generate_new_filename(name: &str, extension: &str, data: &[u8]) -> std::io::Result<String> {
1019+
use crate::proofs::util::sha256_sum;
9621020

963-
let sum = sha256_sum(data);
964-
for index in 0..100_000 {
965-
let name = format!("{}_{}_{}.{}", name, sum, index, extension);
966-
let path = std::path::Path::new(&name);
967-
if !path.try_exists().unwrap_or(true) {
968-
return Ok(name);
1021+
let sum = sha256_sum(data);
1022+
for index in 0..100_000 {
1023+
let name = format!("{}_{}_{}.{}", name, sum, index, extension);
1024+
let path = std::path::Path::new(&name);
1025+
if !path.try_exists().unwrap_or(true) {
1026+
return Ok(name);
1027+
}
9691028
}
1029+
Err(std::io::Error::other("no filename available"))
9701030
}
971-
Err(std::io::Error::other("no filename available"))
9721031
}
9731032

9741033
#[cfg(test)]

‎ledger/src/scan_state/scan_state.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -779,14 +779,13 @@ pub mod transaction_snark {
779779
}
780780

781781
/// https://github.com/MinaProtocol/mina/blob/05c2f73d0f6e4f1341286843814ce02dcb3919e0/src/lib/one_or_two/one_or_two.ml#L54
782-
pub fn zip<B>(a: OneOrTwo<T>, b: OneOrTwo<B>) -> OneOrTwo<(T, B)> {
782+
pub fn zip<B>(a: OneOrTwo<T>, b: OneOrTwo<B>) -> Result<OneOrTwo<(T, B)>, String> {
783783
use OneOrTwo::*;
784784

785785
match (a, b) {
786-
(One(a), One(b)) => One((a, b)),
787-
(Two((a1, a2)), Two((b1, b2))) => Two(((a1, b1), (a2, b2))),
788-
(One(_), Two(_)) => panic!("One_or_two.zip mismatched"),
789-
(Two(_), One(_)) => panic!("One_or_two.zip mismatched"),
786+
(One(a), One(b)) => Ok(One((a, b))),
787+
(Two((a1, a2)), Two((b1, b2))) => Ok(Two(((a1, b1), (a2, b2)))),
788+
(One(_), Two(_)) | (Two(_), One(_)) => Err("One_or_two.zip mismatched".to_string()),
790789
}
791790
}
792791

‎ledger/src/staged_ledger/staged_ledger.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -825,16 +825,16 @@ impl StagedLedger {
825825
let work_count = completed_works.len() as u64;
826826
let jobs_pairs = scan_state.k_work_pairs_for_new_diff(work_count);
827827

828-
let job_msg_proofs: Vec<(AvailableJob, SokMessage, LedgerProof)> = jobs_pairs
829-
.into_iter()
830-
.zip(completed_works)
831-
.flat_map(|(jobs, work)| {
832-
let message = SokMessage::create(work.fee, work.prover);
833-
OneOrTwo::zip(jobs, work.proofs)
834-
.into_map(|(job, proof)| (job, message.clone(), proof))
835-
.into_iter()
836-
})
837-
.collect();
828+
let mut job_msg_proofs =
829+
Vec::<(AvailableJob, SokMessage, LedgerProof)>::with_capacity(work_count as usize * 2);
830+
831+
for (jobs, work) in jobs_pairs.into_iter().zip(completed_works) {
832+
let message = SokMessage::create(work.fee, work.prover);
833+
let jobs = OneOrTwo::zip(jobs, work.proofs)?
834+
.into_map(|(job, proof)| (job, message.clone(), proof))
835+
.into_iter();
836+
job_msg_proofs.extend(jobs);
837+
}
838838

839839
Self::verify(logger, verifier, job_msg_proofs)
840840
}

0 commit comments

Comments
 (0)