Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4472 +/- ##
==========================================
- Coverage 32.92% 32.81% -0.12%
==========================================
Files 493 493
Lines 58294 58294
==========================================
- Hits 19193 19128 -65
- Misses 35768 35825 +57
- Partials 3333 3341 +8 |
❌ 5 Tests Failed:
View the top 3 failed tests by shortest run time
📣 Thoughts on this report? Let Codecov know! | Powered by Codecov |
bragaigor
left a comment
There was a problem hiding this comment.
LGTM, just 2 minor comments
| let (mem, exec) = env.jit_env(); | ||
| Ok(caller_env::wavmio::validate_certificate( | ||
| &mem, | ||
| exec, | ||
| preimage_type, | ||
| hash_ptr, | ||
| )) |
There was a problem hiding this comment.
nitpick: even though logic i the same, if preimage has the wrong type we silently return None which could make debugging a bit harder, so wondering if it's worth to re-add the eprintln msg?
| } | ||
|
|
||
| /// Reads up to 32 bytes of a sequencer inbox message at the given offset. | ||
| pub fn read_inbox_message( |
There was a problem hiding this comment.
nitpick: I was going to suggest to combine this and read_delayed_inbox_message since they are almost identical, but I think I prefer to keep them separate. Simplifying like this would just make readability harder. Will leave up to you
pub fn read_inbox_message(
...
) -> Result<u32, String> {
read_message(mem, io.get_sequencer_message(msg_num), offset, out_ptr, format!("missing sequencer inbox message {msg_num}"))
}
pub fn read_delayed_inbox_message(
...
) -> Result<u32, String> {
read_message(mem, io.get_delayed_message(msg_num), offset, out_ptr, format!("missing delayed inbox message {msg_num}"))
}
fn read_message(
mem: &mut impl MemAccess,
message: Option<&[u8]>,
offset: u32,
out_ptr: GuestPtr,
err: String,
) -> Result<u32, String> {
let message = message.ok_or(err)?;
let offset = offset as usize;
let len = min(32, message.len().saturating_sub(offset));
let read = message.get(offset..(offset + len)).unwrap_or_default();
mem.write_slice(out_ptr, read);
Ok(read.len() as u32)
}| ) -> Result<u32, Escape> { | ||
| let (mut mem, exec) = env.jit_env(); | ||
| let offset = offset as usize; | ||
| ready_hostio(exec)?; |
There was a problem hiding this comment.
Forgot to ask if the addition of ready_hostio here is intentional?
This PR just ports changes done to standard JIT validator done in #4458
closes NIT-4614