Fix review comments

This commit is contained in:
Sergey Kuznetsov
2026-08-17 14:54:24 +01:00
parent 2605b4a78b
commit 37e2f23ee6
6 changed files with 79 additions and 17 deletions

View File

@@ -7,6 +7,7 @@ ignorePaths:
- cmake/**
- LICENSE.md
- .clang-tidy
- src/test/app/wasm_fixtures/*.c
language: en
allowCompoundWords: true # TODO (#6334)
ignoreRandomStrings: true

View File

@@ -188,12 +188,6 @@ host_functions! {
/// Verify `signature` over `message` under `pubkey`. Reads the three regions and
/// answers `1` if the signature is valid, `0` if not, or a negative error.
///
/// GAS DISCREPANCY: this 300 is the value the C-ABI fork registered
/// (`rippled-wasm-host-functions`, WasmVM.cpp), which this port follows. The
/// prior C++ integration in this tree charged 35000 for the same call — 100x
/// more, and closer to the real cost of signature verification. The value is
/// consensus-critical, so confirm which is intended before this ships.
#[gas = 300]
#[wasm_name = "check_sig"]
fn check_signature(

View File

@@ -1,5 +1,6 @@
use crate::region::Region;
use crate::vm::{MAX_FIELD_BYTES, VmState};
use core::ops::Range;
use wasmi::{Caller, Memory};
use xrpl_host_functions::{HostError, HostFunctionSpec, HostFunctions, HostResult};
@@ -273,6 +274,16 @@ pub(crate) fn write_buffered(
const MANTISSA_BYTES: usize = 8;
const EXPONENT_BYTES: usize = 4;
fn check_fits(data: &[u8], range: &Range<usize>, width: usize) -> HostResult<()> {
let region = data
.get(range.clone())
.ok_or(HostError::PointerOutOfBounds)?;
if region.len() < width {
return Err(HostError::BufferTooSmall);
}
Ok(())
}
/// Service `float_to_mant_exp`, the one call that writes two output regions: the host
/// fills the run's output buffer with the mantissa followed by the exponent, and each
/// is copied to its own guest region once every rule has passed.
@@ -313,28 +324,23 @@ pub(crate) fn write_mant_exp(
return Err(HostError::InternalFatal.into());
}
// Copy the mantissa, then the exponent, each only if its whole value fits its
// region — a region too small is `BufferTooSmall`, with nothing written.
let mant_range = mantissa_out.range()?;
check_fits(data, &mant_range, MANTISSA_BYTES)?;
let exp_range = exponent_out.range()?;
check_fits(data, &exp_range, EXPONENT_BYTES)?;
charge_transfer(state, MANTISSA_BYTES + EXPONENT_BYTES)?;
let mant_dst = data
.get_mut(mant_range)
.ok_or(HostError::PointerOutOfBounds)?;
if mant_dst.len() < MANTISSA_BYTES {
return Err(HostError::BufferTooSmall.into());
}
mant_dst[..MANTISSA_BYTES].copy_from_slice(&state.out_buffer[..MANTISSA_BYTES]);
let exp_range = exponent_out.range()?;
let exp_dst = data
.get_mut(exp_range)
.ok_or(HostError::PointerOutOfBounds)?;
if exp_dst.len() < EXPONENT_BYTES {
return Err(HostError::BufferTooSmall.into());
}
exp_dst[..EXPONENT_BYTES]
.copy_from_slice(&state.out_buffer[MANTISSA_BYTES..MANTISSA_BYTES + EXPONENT_BYTES]);
charge_transfer(state, MANTISSA_BYTES + EXPONENT_BYTES)?;
#[expect(
clippy::cast_possible_truncation,
clippy::cast_possible_wrap,

View File

@@ -658,6 +658,42 @@ fn a_modest_run_never_meets_the_budget() {
assert_eq!(outcome.result, MAX_FIELD_BYTES as i32);
}
/// A write the budget refuses is a write that did not happen. `float_to_mant_exp` is
/// the case worth pinning: its two regions are charged as one, so a call that cannot
/// pay for both must leave both alone rather than place the mantissa and refuse.
#[test]
fn a_write_the_budget_refuses_reaches_guest_memory_in_no_part() {
let host = FakeHost::new()
.answering_field(1, Answer::filler(MAX_FIELD_BYTES))
.answering_float_mant_exp(vec![1, 2, 3, 4, 5, 6, 7, 8], vec![9, 10, 11, 12]);
// Spend the budget on 1 KiB fields at offset 0, then ask for a mantissa and an
// exponent at offsets well clear of them.
let call = "(call $float_to_mant_exp (i32.const 0) (i32.const 8) (i32.const 2048) (i32.const 8) (i32.const 2064) (i32.const 4))";
let spent = |tail: &str| {
module(
&[import::HOME_LE_FIELD, import::FLOAT_TO_MANT_EXP, ONE_PAGE],
&format!(
"(local $r i32)
(loop $l
(local.set $r (call $home_le_field (i32.const 1) (i32.const 0) (i32.const {MAX_FIELD_BYTES})))
(br_if $l {WHILE_POSITIVE}))
{tail}"
),
)
};
let refused = run(&spent(call), &host).expect("the module should run");
assert_eq!(refused.result, code(HostError::OutOfTransferLimit));
let wat = spent(&format!(
"(drop {call})
(i32.or (i32.load8_u (i32.const 2048)) (i32.load8_u (i32.const 2064)))"
));
let outcome = run(&wat, &host).expect("the module should run");
assert_eq!(outcome.result, 0, "neither region should be written");
}
/// Reads leave the budget alone: `read_borrowed` hands the host a slice *aliasing*
/// guest memory, so there are no copied bytes to charge. What bounds how many reads
/// a run can make is gas, which every host call pays before its body runs.

View File

@@ -953,6 +953,30 @@ fn float_to_mant_exp_with_a_wrong_total_stops_the_run() {
);
}
/// The two regions are one answer, so a call that cannot place all of it places none
/// of it: an exponent region too small refuses the call with the mantissa's own region
/// wide enough and untouched.
#[test]
fn float_to_mant_exp_with_a_short_exponent_region_writes_neither() {
let host =
FakeHost::new().answering_float_mant_exp(vec![1, 2, 3, 4, 5, 6, 7, 8], vec![9, 10, 11, 12]);
// Eight bytes for the mantissa at offset 64, but two for the exponent at 80.
let call = "(call $float_to_mant_exp (i32.const 0) (i32.const 8) (i32.const 64) (i32.const 8) (i32.const 80) (i32.const 2))";
let wat = module(&[import::FLOAT_TO_MANT_EXP, ONE_PAGE], call);
assert_eq!(status(&wat, &host), code(HostError::BufferTooSmall));
let wat = module(
&[import::FLOAT_TO_MANT_EXP, ONE_PAGE],
&format!(
"(drop {call})
(i32.or (i32.load8_u (i32.const 64)) (i32.load8_u (i32.const 80)))"
),
);
assert_eq!(status(&wat, &host), 0, "neither region should be written");
}
/// A comparison that reads two float regions and returns a scalar verdict, no output
/// region involved.
#[test]

View File

@@ -50,6 +50,7 @@ answer(rust::Slice<std::uint8_t> out, std::uint8_t const* value, std::size_t siz
{
std::memcpy(out.data(), value, size);
}
size = std::min(size, static_cast<std::size_t>(std::numeric_limits<std::int32_t>::max()));
return static_cast<std::int32_t>(size);
}