Skip to content

Commit 0cf1f66

Browse files
chipshortchipshort
committed
Make Logs log directly
1 parent 4202e7f commit 0cf1f66

File tree

3 files changed

+61
-121
lines changed

3 files changed

+61
-121
lines changed

‎packages/check/src/main.rs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use clap::{Arg, ArgAction, Command};
88
use colored::Colorize;
99

1010
use cosmwasm_vm::capabilities_from_csv;
11-
use cosmwasm_vm::internals::{check_wasm_with_logs, compile, make_compiling_engine, Logs};
11+
use cosmwasm_vm::internals::{check_wasm_with_logs, compile, make_compiling_engine, Logger};
1212

1313
const DEFAULT_AVAILABLE_CAPABILITIES: &str =
1414
"iterator,staking,stargate,cosmwasm_1_1,cosmwasm_1_2,cosmwasm_1_3,cosmwasm_1_4,cosmwasm_2_0,cosmwasm_2_1";
@@ -103,13 +103,9 @@ fn check_contract(
103103
let mut wasm = Vec::<u8>::new();
104104
file.read_to_end(&mut wasm)?;
105105

106-
let mut logs = if verbose { Logs::new() } else { Logs::Off };
106+
let logs = if verbose { Logger::new() } else { Logger::Off };
107107
// Check wasm
108-
let res = check_wasm_with_logs(&wasm, available_capabilities, &mut logs);
109-
for line in logs {
110-
eprintln!("{}", line);
111-
}
112-
res?;
108+
check_wasm_with_logs(&wasm, available_capabilities, logs)?;
113109

114110
// Compile module
115111
let engine = make_compiling_engine(None);

‎packages/vm/src/compatibility.rs

Lines changed: 57 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
use std::cell::RefCell;
2-
use std::cell::RefMut;
31
use std::collections::BTreeSet;
42
use std::collections::HashSet;
53

@@ -99,70 +97,59 @@ const MAX_TOTAL_FUNCTION_PARAMS: usize = 10_000;
9997
/// during static validation.
10098
const MAX_FUNCTION_RESULTS: usize = 1;
10199

102-
#[derive(Clone, Default)]
103-
pub enum Logs {
104-
On(RefCell<Vec<String>>),
100+
#[derive(Clone, Copy)]
101+
pub enum LogOutput {
102+
StdOut,
103+
StdErr,
104+
}
105+
#[derive(Clone, Copy, Default)]
106+
pub enum Logger<'a> {
107+
On {
108+
prefix: &'a str,
109+
output: LogOutput,
110+
},
105111
#[default]
106112
Off,
107113
}
108114

109-
pub struct LogAccess<'a>(RefMut<'a, Vec<String>>);
110-
111-
impl<'a> LogAccess<'a> {
112-
/// Adds a message to the logs
113-
pub fn add(&mut self, msg: impl Into<String>) {
114-
self.0.push(msg.into());
115-
}
116-
}
117-
118-
impl Logs {
115+
impl<'a> Logger<'a> {
119116
pub fn new() -> Self {
120-
On(RefCell::new(Vec::new()))
117+
On {
118+
output: LogOutput::StdOut,
119+
prefix: "",
120+
}
121121
}
122122

123-
// Gets access to logs for writing.
124-
// Use this if you want to add multiple messages at once.
125-
pub fn open(&mut self) -> Option<LogAccess> {
126-
match self {
127-
On(data) => Some(LogAccess(data.borrow_mut())),
128-
Off => None,
129-
}
123+
pub fn with_config(output: LogOutput, prefix: &'a str) -> Self {
124+
On { output, prefix }
130125
}
131126

132127
/// Adds a message to the logs, if they are enabled.
133128
/// This is a convenience method for adding a single message.
134129
///
135130
/// Takes a closure that returns the message to add to avoid unnecessary allocations.
136-
pub fn add(&mut self, msg_fn: impl FnOnce() -> String) {
137-
if let Some(mut logs) = self.open() {
138-
logs.add(msg_fn());
139-
}
140-
}
141-
}
142-
143-
impl IntoIterator for Logs {
144-
type Item = String;
145-
type IntoIter = std::vec::IntoIter<Self::Item>;
146-
147-
fn into_iter(self) -> Self::IntoIter {
148-
match self {
149-
Logs::On(data) => data.into_inner().into_iter(),
150-
Logs::Off => vec![].into_iter(),
131+
pub fn add(&self, msg_fn: impl FnOnce() -> String) {
132+
if let On { prefix, output } = &self {
133+
let msg = msg_fn();
134+
match output {
135+
LogOutput::StdOut => println!("{prefix}{msg}"),
136+
LogOutput::StdErr => eprintln!("{prefix}{msg}"),
137+
}
151138
}
152139
}
153140
}
154141

155-
use Logs::*;
142+
use Logger::*;
156143

157144
/// Checks if the data is valid wasm and compatibility with the CosmWasm API (imports and exports)
158145
pub fn check_wasm(wasm_code: &[u8], available_capabilities: &HashSet<String>) -> VmResult<()> {
159-
check_wasm_with_logs(wasm_code, available_capabilities, &mut Off)
146+
check_wasm_with_logs(wasm_code, available_capabilities, Off)
160147
}
161148

162-
pub fn check_wasm_with_logs(
149+
pub fn check_wasm_with_logs<'a>(
163150
wasm_code: &[u8],
164151
available_capabilities: &HashSet<String>,
165-
logs: &mut Logs,
152+
logs: Logger<'a>,
166153
) -> VmResult<()> {
167154
logs.add(|| format!("Size of Wasm blob: {}", wasm_code.len()));
168155

@@ -255,7 +242,7 @@ fn check_interface_version(module: &ParsedWasm) -> VmResult<()> {
255242
}
256243
}
257244

258-
fn check_wasm_exports(module: &ParsedWasm, logs: &mut Logs) -> VmResult<()> {
245+
fn check_wasm_exports(module: &ParsedWasm, logs: Logger) -> VmResult<()> {
259246
let available_exports: HashSet<String> = module.exported_function_names(None);
260247

261248
logs.add(|| format!("Exports: {}", available_exports.to_string_limited(20_000)));
@@ -276,7 +263,7 @@ fn check_wasm_exports(module: &ParsedWasm, logs: &mut Logs) -> VmResult<()> {
276263
fn check_wasm_imports(
277264
module: &ParsedWasm,
278265
supported_imports: &[&str],
279-
logs: &mut Logs,
266+
logs: Logger,
280267
) -> VmResult<()> {
281268
logs.add(|| {
282269
format!(
@@ -327,7 +314,7 @@ fn full_import_name(ie: &Import) -> String {
327314
fn check_wasm_capabilities(
328315
module: &ParsedWasm,
329316
available_capabilities: &HashSet<String>,
330-
logs: &mut Logs,
317+
logs: Logger,
331318
) -> VmResult<()> {
332319
let required_capabilities = required_capabilities_from_module(module);
333320
logs.add(|| {
@@ -349,19 +336,16 @@ fn check_wasm_capabilities(
349336
Ok(())
350337
}
351338

352-
fn check_wasm_functions(module: &ParsedWasm, logs: &mut Logs) -> VmResult<()> {
353-
if let Some(mut logs) = logs.open() {
354-
logs.add(format!("Function count: {}", module.function_count));
355-
logs.add(format!(
356-
"Max function parameters: {}",
357-
module.max_func_params
358-
));
359-
logs.add(format!("Max function results: {}", module.max_func_results));
360-
logs.add(format!(
339+
fn check_wasm_functions(module: &ParsedWasm, logs: Logger) -> VmResult<()> {
340+
logs.add(|| format!("Function count: {}", module.function_count));
341+
logs.add(|| format!("Max function parameters: {}", module.max_func_params));
342+
logs.add(|| format!("Max function results: {}", module.max_func_results));
343+
logs.add(|| {
344+
format!(
361345
"Total function parameter count: {}",
362346
module.total_func_params
363-
));
364-
}
347+
)
348+
});
365349

366350
if module.function_count > MAX_FUNCTIONS {
367351
return Err(VmError::static_validation_err(format!(
@@ -405,29 +389,6 @@ mod tests {
405389
capabilities_from_csv("cosmwasm_1_1,cosmwasm_1_2,cosmwasm_1_3,iterator,staking,stargate")
406390
}
407391

408-
#[test]
409-
fn logs_works() {
410-
let mut logs = Logs::new();
411-
412-
logs.add(|| "a test".to_string());
413-
414-
if let Some(mut logs) = logs.open() {
415-
logs.add("second test");
416-
logs.add("third test");
417-
}
418-
419-
let mut logs_b = logs.clone();
420-
if let Some(mut logs) = logs_b.open() {
421-
logs.add("added in b");
422-
}
423-
424-
let mut iter = logs.into_iter();
425-
assert_eq!(iter.next(), Some(String::from("a test")));
426-
assert_eq!(iter.next(), Some(String::from("second test")));
427-
assert_eq!(iter.next(), Some(String::from("third test")));
428-
assert_eq!(iter.next(), None);
429-
}
430-
431392
#[test]
432393
fn check_wasm_passes_for_latest_contract() {
433394
// this is our reference check, must pass
@@ -729,7 +690,7 @@ mod tests {
729690
)
730691
.unwrap();
731692
let module = ParsedWasm::parse(&wasm).unwrap();
732-
check_wasm_exports(&module, &mut Off).unwrap();
693+
check_wasm_exports(&module, Off).unwrap();
733694

734695
// this is invalid, as it doesn't any required export
735696
let wasm = wat::parse_str(
@@ -741,7 +702,7 @@ mod tests {
741702
)
742703
.unwrap();
743704
let module = ParsedWasm::parse(&wasm).unwrap();
744-
match check_wasm_exports(&module, &mut Off) {
705+
match check_wasm_exports(&module, Off) {
745706
Err(VmError::StaticValidationErr { msg, .. }) => {
746707
assert!(msg.starts_with("Wasm contract doesn't have required export: \"allocate\""));
747708
}
@@ -760,7 +721,7 @@ mod tests {
760721
)
761722
.unwrap();
762723
let module = ParsedWasm::parse(&wasm).unwrap();
763-
match check_wasm_exports(&module, &mut Off) {
724+
match check_wasm_exports(&module, Off) {
764725
Err(VmError::StaticValidationErr { msg, .. }) => {
765726
assert!(
766727
msg.starts_with("Wasm contract doesn't have required export: \"deallocate\"")
@@ -790,12 +751,7 @@ mod tests {
790751
)"#,
791752
)
792753
.unwrap();
793-
check_wasm_imports(
794-
&ParsedWasm::parse(&wasm).unwrap(),
795-
SUPPORTED_IMPORTS,
796-
&mut Off,
797-
)
798-
.unwrap();
754+
check_wasm_imports(&ParsedWasm::parse(&wasm).unwrap(), SUPPORTED_IMPORTS, Off).unwrap();
799755
}
800756

801757
#[test]
@@ -906,12 +862,8 @@ mod tests {
906862
)"#,
907863
)
908864
.unwrap();
909-
let err = check_wasm_imports(
910-
&ParsedWasm::parse(&wasm).unwrap(),
911-
SUPPORTED_IMPORTS,
912-
&mut Off,
913-
)
914-
.unwrap_err();
865+
let err = check_wasm_imports(&ParsedWasm::parse(&wasm).unwrap(), SUPPORTED_IMPORTS, Off)
866+
.unwrap_err();
915867
match err {
916868
VmError::StaticValidationErr { msg, .. } => {
917869
assert_eq!(msg, "Import count exceeds limit. Imports: 101. Limit: 100.");
@@ -948,11 +900,7 @@ mod tests {
948900
"env.debug",
949901
"env.query_chain",
950902
];
951-
let result = check_wasm_imports(
952-
&ParsedWasm::parse(&wasm).unwrap(),
953-
supported_imports,
954-
&mut Off,
955-
);
903+
let result = check_wasm_imports(&ParsedWasm::parse(&wasm).unwrap(), supported_imports, Off);
956904
match result.unwrap_err() {
957905
VmError::StaticValidationErr { msg, .. } => {
958906
println!("{msg}");
@@ -968,7 +916,7 @@ mod tests {
968916
#[test]
969917
fn check_wasm_imports_of_old_contract() {
970918
let module = &ParsedWasm::parse(CONTRACT_0_7).unwrap();
971-
let result = check_wasm_imports(module, SUPPORTED_IMPORTS, &mut Off);
919+
let result = check_wasm_imports(module, SUPPORTED_IMPORTS, Off);
972920
match result.unwrap_err() {
973921
VmError::StaticValidationErr { msg, .. } => {
974922
assert!(
@@ -982,11 +930,7 @@ mod tests {
982930
#[test]
983931
fn check_wasm_imports_wrong_type() {
984932
let wasm = wat::parse_str(r#"(module (import "env" "db_read" (memory 1 1)))"#).unwrap();
985-
let result = check_wasm_imports(
986-
&ParsedWasm::parse(&wasm).unwrap(),
987-
SUPPORTED_IMPORTS,
988-
&mut Off,
989-
);
933+
let result = check_wasm_imports(&ParsedWasm::parse(&wasm).unwrap(), SUPPORTED_IMPORTS, Off);
990934
match result.unwrap_err() {
991935
VmError::StaticValidationErr { msg, .. } => {
992936
assert!(
@@ -1021,7 +965,7 @@ mod tests {
1021965
]
1022966
.into_iter()
1023967
.collect();
1024-
check_wasm_capabilities(&module, &available, &mut Off).unwrap();
968+
check_wasm_capabilities(&module, &available, Off).unwrap();
1025969
}
1026970

1027971
#[test]
@@ -1049,7 +993,7 @@ mod tests {
1049993
]
1050994
.into_iter()
1051995
.collect();
1052-
match check_wasm_capabilities(&module, &available, &mut Off).unwrap_err() {
996+
match check_wasm_capabilities(&module, &available, Off).unwrap_err() {
1053997
VmError::StaticValidationErr { msg, .. } => assert_eq!(
1054998
msg,
1055999
"Wasm contract requires unavailable capabilities: {\"sun\"}"
@@ -1065,7 +1009,7 @@ mod tests {
10651009
]
10661010
.into_iter()
10671011
.collect();
1068-
match check_wasm_capabilities(&module, &available, &mut Off).unwrap_err() {
1012+
match check_wasm_capabilities(&module, &available, Off).unwrap_err() {
10691013
VmError::StaticValidationErr { msg, .. } => assert_eq!(
10701014
msg,
10711015
"Wasm contract requires unavailable capabilities: {\"sun\", \"water\"}"
@@ -1075,7 +1019,7 @@ mod tests {
10751019

10761020
// Available set 3
10771021
let available = ["freedom".to_string()].into_iter().collect();
1078-
match check_wasm_capabilities(&module, &available, &mut Off).unwrap_err() {
1022+
match check_wasm_capabilities(&module, &available, Off).unwrap_err() {
10791023
VmError::StaticValidationErr { msg, .. } => assert_eq!(
10801024
msg,
10811025
"Wasm contract requires unavailable capabilities: {\"nutrients\", \"sun\", \"water\"}"
@@ -1085,7 +1029,7 @@ mod tests {
10851029

10861030
// Available set 4
10871031
let available = [].into_iter().collect();
1088-
match check_wasm_capabilities(&module, &available, &mut Off).unwrap_err() {
1032+
match check_wasm_capabilities(&module, &available, Off).unwrap_err() {
10891033
VmError::StaticValidationErr { msg, .. } => assert_eq!(
10901034
msg,
10911035
"Wasm contract requires unavailable capabilities: {\"nutrients\", \"sun\", \"water\"}"
@@ -1107,7 +1051,7 @@ mod tests {
11071051
.unwrap();
11081052
let module = ParsedWasm::parse(&wasm).unwrap();
11091053

1110-
match check_wasm_functions(&module, &mut Off).unwrap_err() {
1054+
match check_wasm_functions(&module, Off).unwrap_err() {
11111055
VmError::StaticValidationErr { msg, .. } => assert_eq!(
11121056
msg,
11131057
"Wasm contract contains function with more than 100 parameters"
@@ -1126,7 +1070,7 @@ mod tests {
11261070
))
11271071
.unwrap();
11281072
let module = ParsedWasm::parse(&wasm).unwrap();
1129-
match check_wasm_functions(&module, &mut Off).unwrap_err() {
1073+
match check_wasm_functions(&module, Off).unwrap_err() {
11301074
VmError::StaticValidationErr { msg, .. } => assert_eq!(
11311075
msg,
11321076
"Wasm contract contains function with more than 1 results"
@@ -1145,7 +1089,7 @@ mod tests {
11451089
))
11461090
.unwrap();
11471091
let module = ParsedWasm::parse(&wasm).unwrap();
1148-
match check_wasm_functions(&module, &mut Off).unwrap_err() {
1092+
match check_wasm_functions(&module, Off).unwrap_err() {
11491093
VmError::StaticValidationErr { msg, .. } => {
11501094
assert_eq!(msg, "Wasm contract contains more than 20000 functions")
11511095
}

‎packages/vm/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ pub mod internals {
5555
//! Please don't use any of these types directly, as
5656
//! they might change frequently or be removed in the future.
5757
58-
pub use crate::compatibility::{check_wasm, check_wasm_with_logs, Logs};
58+
pub use crate::compatibility::{check_wasm, check_wasm_with_logs, LogOutput, Logger};
5959
pub use crate::instance::instance_from_module;
6060
pub use crate::wasm_backend::{compile, make_compiling_engine, make_runtime_engine};
6161
}

0 commit comments

Comments
 (0)