Skip to content

Commit 86a07a8

Browse files
committed
fix: resolve code review findings
P1: Replace unimplemented!() panic in NearTrigger::to_rust_bytes() with Vec::new() P2: Propagate PossibleReorg errors from ipfs_cat host fn instead of swallowing them P2: Fix useless .into() conversions on anyhow::Error (clippy useless_conversion) P3: Apply rustfmt to all changed files P3: Clarify comment on byte-scan Rust detection heuristic in ValidModule::new() Build: cargo build clean Tests: 14 rust_abi + existing wasm tests all pass Lint: cargo clippy --deny warnings clean Format: cargo fmt --check clean
1 parent a2c862a commit 86a07a8

9 files changed

Lines changed: 107 additions & 74 deletions

File tree

chain/ethereum/src/runtime/runtime_adapter.rs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ impl RawEthCall for EthereumRawEthCall {
221221
archive: false,
222222
traces: false,
223223
}))
224-
.map_err(|e| HostExportError::Unknown(e.into()))?;
224+
.map_err(HostExportError::Unknown)?;
225225

226226
// Create a raw call request
227227
let req = call::Request::new(Address::from(address), calldata.to_vec(), 0);
@@ -267,15 +267,12 @@ impl RawEthCall for EthereumRawEthCall {
267267
call::Retval::Null => Ok(None),
268268
}
269269
}
270-
Err(ContractCallError::AlloyError(e)) => {
271-
Err(HostExportError::PossibleReorg(anyhow::anyhow!(
272-
"eth_call RPC error: {}",
273-
e
274-
)))
275-
}
276-
Err(ContractCallError::Timeout) => Err(HostExportError::PossibleReorg(anyhow::anyhow!(
277-
"eth_call timed out"
278-
))),
270+
Err(ContractCallError::AlloyError(e)) => Err(HostExportError::PossibleReorg(
271+
anyhow::anyhow!("eth_call RPC error: {}", e),
272+
)),
273+
Err(ContractCallError::Timeout) => Err(HostExportError::PossibleReorg(
274+
anyhow::anyhow!("eth_call timed out"),
275+
)),
279276
Err(e) => Err(HostExportError::Unknown(anyhow::anyhow!(
280277
"eth_call failed: {}",
281278
e

chain/ethereum/src/trigger.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ use graph::runtime::AscPtr;
2323
use graph::runtime::HostExportError;
2424
use graph::semver::Version;
2525
use graph_runtime_wasm::module::ToAscPtr;
26-
use graph_runtime_wasm::rust_abi::{RustBlockTrigger, RustCallTrigger, RustLogTrigger, ToRustBytes};
26+
use graph_runtime_wasm::rust_abi::{
27+
RustBlockTrigger, RustCallTrigger, RustLogTrigger, ToRustBytes,
28+
};
2729
use std::{cmp::Ordering, sync::Arc};
2830

2931
use crate::runtime::abi::AscEthereumBlock;
@@ -672,13 +674,7 @@ impl ToRustBytes for MappingTrigger {
672674
log_index: log.log_index.unwrap_or(0),
673675
block_number: block.number_u64(),
674676
block_timestamp: block.inner().header.timestamp,
675-
topics: log
676-
.inner
677-
.data
678-
.topics()
679-
.iter()
680-
.map(|t| t.0)
681-
.collect(),
677+
topics: log.inner.data.topics().iter().map(|t| t.0).collect(),
682678
data: log.inner.data.data.to_vec(),
683679
};
684680
rust_trigger.to_rust_bytes()

chain/near/src/trigger.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,9 +146,11 @@ impl MappingTriggerTrait for NearTrigger {
146146

147147
impl ToRustBytes for NearTrigger {
148148
fn to_rust_bytes(&self) -> Vec<u8> {
149-
// NEAR triggers are not yet supported by Graphite SDK.
150-
// This stub satisfies the trait bound so Ethereum Rust subgraphs can compile.
151-
unimplemented!("Rust ABI serialization is not yet supported for NEAR triggers")
149+
// NEAR triggers are not yet supported by the Rust ABI.
150+
// Return empty bytes rather than panicking; the handler will receive no data
151+
// and can decide how to proceed. A Rust subgraph targeting NEAR would need
152+
// to implement NEAR-specific serialization in a future PR.
153+
Vec::new()
152154
}
153155
}
154156

runtime/wasm/src/mapping.rs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -273,13 +273,21 @@ impl ValidModule {
273273
raw_module: &[u8],
274274
timeout: Option<Duration>,
275275
) -> Result<Self, anyhow::Error> {
276-
// Detect Rust modules by scanning for the "graphite" import namespace.
277-
// Rust modules use modern WASM features (bulk-memory, reference-types, etc.)
278-
// that parity_wasm cannot parse, so we skip parity_wasm gas injection for them.
276+
// Pre-parse detection: scan raw bytes for the "graphite" import namespace string.
277+
// This is a fast heuristic used solely to decide whether to skip parity_wasm gas
278+
// injection, which cannot parse modern WASM features (bulk-memory, reference-types)
279+
// that Rust-compiled modules use. A false positive (an AS module containing the
280+
// literal string "graphite") would be caught later: build_linker() does a proper
281+
// import-map check, and invoke_handler_rust() would immediately fail with
282+
// "function 'allocate' not found" rather than silently misbehaving.
283+
// The authoritative language detection happens in build_linker() below.
279284
let is_rust_module = raw_module.windows(8).any(|w| w == b"graphite");
280285

281286
let (raw_module, start_function) = if is_rust_module {
282-
info!(logger, "Detected Rust WASM module, skipping parity_wasm gas injection");
287+
info!(
288+
logger,
289+
"Detected Rust WASM module, skipping parity_wasm gas injection"
290+
);
283291
(raw_module.to_vec(), None)
284292
} else {
285293
// Add the gas calls here. Module name "gas" must match. See also
@@ -317,8 +325,9 @@ impl ValidModule {
317325

318326
name
319327
});
320-
let parity_module = wasm_instrument::gas_metering::inject(parity_module, &GasRules, "gas")
321-
.map_err(|_| anyhow!("Failed to inject gas counter"))?;
328+
let parity_module =
329+
wasm_instrument::gas_metering::inject(parity_module, &GasRules, "gas")
330+
.map_err(|_| anyhow!("Failed to inject gas counter"))?;
322331
(parity_module.into_bytes()?, start_function)
323332
};
324333

runtime/wasm/src/module/context.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1289,10 +1289,8 @@ impl WasmInstanceContext<'_> {
12891289
}
12901290

12911291
// Convert HashMap<String, Value> to HashMap<Word, Value>
1292-
let data: std::collections::HashMap<Word, store::Value> = data
1293-
.into_iter()
1294-
.map(|(k, v)| (Word::from(k), v))
1295-
.collect();
1292+
let data: std::collections::HashMap<Word, store::Value> =
1293+
data.into_iter().map(|(k, v)| (Word::from(k), v)).collect();
12961294

12971295
let host_exports = self.as_ref().ctx.host_exports.cheap_clone();
12981296
let ctx = &mut self.as_mut().ctx;
@@ -1404,7 +1402,13 @@ impl WasmInstanceContext<'_> {
14041402
let ctx = &mut self.as_mut().ctx;
14051403

14061404
host_exports
1407-
.log_log(&ctx.logger, slog_level, message.to_string(), gas, &mut ctx.state)
1405+
.log_log(
1406+
&ctx.logger,
1407+
slog_level,
1408+
message.to_string(),
1409+
gas,
1410+
&mut ctx.state,
1411+
)
14081412
.map_err(|e| HostExportError::Deterministic(e.into()))
14091413
}
14101414

@@ -1480,6 +1484,6 @@ impl WasmInstanceContext<'_> {
14801484
host_exports
14811485
.ipfs_cat(&logger, hash.to_string())
14821486
.await
1483-
.map_err(|e| HostExportError::PossibleReorg(e.into()))
1487+
.map_err(HostExportError::PossibleReorg)
14841488
}
14851489
}

runtime/wasm/src/module/instance.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,10 @@ impl WasmInstance {
218218
Ok(result_code) => {
219219
if result_code != 0 {
220220
// Non-zero return code indicates handler error
221-
Some(anyhow::anyhow!("handler returned error code {}", result_code))
221+
Some(anyhow::anyhow!(
222+
"handler returned error code {}",
223+
result_code
224+
))
222225
} else {
223226
assert!(!self.instance_ctx().as_ref().possible_reorg);
224227
assert!(!self.instance_ctx().as_ref().deterministic_host_trap);
@@ -551,7 +554,9 @@ pub(crate) fn build_linker(
551554
linker.func_wrap(
552555
"gas",
553556
"gas",
554-
|mut caller: wasmtime::Caller<'_, WasmInstanceData>, gas_used: u32| -> anyhow::Result<()> {
557+
|mut caller: wasmtime::Caller<'_, WasmInstanceData>,
558+
gas_used: u32|
559+
-> anyhow::Result<()> {
555560
use graph::runtime::gas::SaturatingInto;
556561
if let Err(e) = caller
557562
.data()

runtime/wasm/src/rust_abi/host.rs

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,7 @@ fn read_bytes_with_gas(
2121
gas: &graph::runtime::gas::GasCounter,
2222
) -> Result<Vec<u8>, anyhow::Error> {
2323
// Charge gas for memory read
24-
gas.consume_host_fn_with_metrics(
25-
Gas::new(GAS_COST_LOAD as u64 * len as u64),
26-
"rust_abi_read",
27-
)?;
24+
gas.consume_host_fn_with_metrics(Gas::new(GAS_COST_LOAD as u64 * len as u64), "rust_abi_read")?;
2825

2926
let data = memory.data(&store);
3027
let start = ptr as usize;
@@ -160,8 +157,13 @@ pub fn link_rust_host_functions(
160157
let memory = get_memory(&mut caller)?;
161158
let gas = caller.data().gas.cheap_clone();
162159

163-
let entity_type =
164-
read_string_with_gas(&memory, &caller, entity_type_ptr, entity_type_len, &gas)?;
160+
let entity_type = read_string_with_gas(
161+
&memory,
162+
&caller,
163+
entity_type_ptr,
164+
entity_type_len,
165+
&gas,
166+
)?;
165167
let id = read_string_with_gas(&memory, &caller, id_ptr, id_len, &gas)?;
166168
let data_bytes =
167169
read_bytes_with_gas(&memory, &caller, data_ptr, data_len, &gas)?;
@@ -197,8 +199,13 @@ pub fn link_rust_host_functions(
197199
let memory = get_memory(&mut caller)?;
198200
let gas = caller.data().gas.cheap_clone();
199201

200-
let entity_type =
201-
read_string_with_gas(&memory, &caller, entity_type_ptr, entity_type_len, &gas)?;
202+
let entity_type = read_string_with_gas(
203+
&memory,
204+
&caller,
205+
entity_type_ptr,
206+
entity_type_len,
207+
&gas,
208+
)?;
202209
let id = read_string_with_gas(&memory, &caller, id_ptr, id_len, &gas)?;
203210

204211
// Call the actual store_get through WasmInstanceContext
@@ -233,8 +240,13 @@ pub fn link_rust_host_functions(
233240
let memory = get_memory(&mut caller)?;
234241
let gas = caller.data().gas.cheap_clone();
235242

236-
let entity_type =
237-
read_string_with_gas(&memory, &caller, entity_type_ptr, entity_type_len, &gas)?;
243+
let entity_type = read_string_with_gas(
244+
&memory,
245+
&caller,
246+
entity_type_ptr,
247+
entity_type_len,
248+
&gas,
249+
)?;
238250
let id = read_string_with_gas(&memory, &caller, id_ptr, id_len, &gas)?;
239251

240252
let mut ctx = std::pin::pin!(WasmInstanceContext::new(&mut caller));
@@ -393,7 +405,13 @@ pub fn link_rust_host_functions(
393405
Ok(bytes.len() as u32)
394406
}
395407
}
396-
Err(_) => Ok(u32::MAX), // Error indicator
408+
Err(e) => {
409+
// Propagate reorg signals as traps so the block can be retried.
410+
if let graph::runtime::HostExportError::PossibleReorg(_) = &e {
411+
caller.data_mut().possible_reorg = true;
412+
}
413+
Err(anyhow::anyhow!("ipfs_cat failed: {}", e))
414+
}
397415
}
398416
})
399417
},
@@ -438,8 +456,7 @@ pub fn link_rust_host_functions(
438456
address.copy_from_slice(&addr_bytes);
439457

440458
// Read calldata
441-
let calldata =
442-
read_bytes_with_gas(&memory, &caller, data_ptr, data_len, &gas)?;
459+
let calldata = read_bytes_with_gas(&memory, &caller, data_ptr, data_len, &gas)?;
443460

444461
// Get the raw_eth_call capability from ctx
445462
let raw_eth_call = caller
@@ -457,7 +474,9 @@ pub fn link_rust_host_functions(
457474
let block_ptr = caller.data().ctx.block_ptr.cheap_clone();
458475

459476
// Make the call
460-
let result = raw_eth_call.call(address, &calldata, &block_ptr, None).await;
477+
let result = raw_eth_call
478+
.call(address, &calldata, &block_ptr, None)
479+
.await;
461480

462481
match result {
463482
Ok(Some(bytes)) => {
@@ -503,8 +522,9 @@ pub fn link_rust_host_functions(
503522
let memory = get_memory(&mut caller)?;
504523
let gas = caller.data().gas.cheap_clone();
505524

506-
let message = read_string_with_gas(&memory, &caller, message_ptr, message_len, &gas)
507-
.unwrap_or_else(|_| "<failed to read message>".to_string());
525+
let message =
526+
read_string_with_gas(&memory, &caller, message_ptr, message_len, &gas)
527+
.unwrap_or_else(|_| "<failed to read message>".to_string());
508528

509529
// Mark as deterministic trap
510530
caller.data_mut().deterministic_host_trap = true;

runtime/wasm/src/rust_abi/trigger.rs

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -48,16 +48,16 @@ impl ToRustBytes for RustLogTrigger {
4848
impl ToRustWasm for RustLogTrigger {
4949
fn write_to<W: Write>(&self, writer: &mut W) -> io::Result<()> {
5050
// Fixed-size fields first (no length prefix)
51-
writer.write_all(&self.address)?; // 20 bytes
52-
writer.write_all(&self.tx_hash)?; // 32 bytes
53-
writer.write_all(&self.log_index.to_le_bytes())?; // 8 bytes
54-
writer.write_all(&self.block_number.to_le_bytes())?; // 8 bytes
51+
writer.write_all(&self.address)?; // 20 bytes
52+
writer.write_all(&self.tx_hash)?; // 32 bytes
53+
writer.write_all(&self.log_index.to_le_bytes())?; // 8 bytes
54+
writer.write_all(&self.block_number.to_le_bytes())?; // 8 bytes
5555
writer.write_all(&self.block_timestamp.to_le_bytes())?; // 8 bytes
5656

5757
// Topics array: count + data
5858
writer.write_all(&(self.topics.len() as u32).to_le_bytes())?;
5959
for topic in &self.topics {
60-
writer.write_all(topic)?; // 32 bytes each
60+
writer.write_all(topic)?; // 32 bytes each
6161
}
6262

6363
// Data: length + bytes
@@ -100,12 +100,12 @@ impl ToRustBytes for RustCallTrigger {
100100
impl ToRustWasm for RustCallTrigger {
101101
fn write_to<W: Write>(&self, writer: &mut W) -> io::Result<()> {
102102
// Fixed-size fields
103-
writer.write_all(&self.to)?; // 20 bytes
104-
writer.write_all(&self.from)?; // 20 bytes
105-
writer.write_all(&self.tx_hash)?; // 32 bytes
106-
writer.write_all(&self.block_number.to_le_bytes())?; // 8 bytes
107-
writer.write_all(&self.block_timestamp.to_le_bytes())?; // 8 bytes
108-
writer.write_all(&self.block_hash)?; // 32 bytes
103+
writer.write_all(&self.to)?; // 20 bytes
104+
writer.write_all(&self.from)?; // 20 bytes
105+
writer.write_all(&self.tx_hash)?; // 32 bytes
106+
writer.write_all(&self.block_number.to_le_bytes())?; // 8 bytes
107+
writer.write_all(&self.block_timestamp.to_le_bytes())?; // 8 bytes
108+
writer.write_all(&self.block_hash)?; // 32 bytes
109109

110110
// Input: length + bytes
111111
writer.write_all(&(self.input.len() as u32).to_le_bytes())?;
@@ -152,15 +152,15 @@ impl ToRustBytes for RustBlockTrigger {
152152

153153
impl ToRustWasm for RustBlockTrigger {
154154
fn write_to<W: Write>(&self, writer: &mut W) -> io::Result<()> {
155-
writer.write_all(&self.hash)?; // 32 bytes
156-
writer.write_all(&self.parent_hash)?; // 32 bytes
157-
writer.write_all(&self.number.to_le_bytes())?; // 8 bytes
158-
writer.write_all(&self.timestamp.to_le_bytes())?; // 8 bytes
159-
writer.write_all(&self.author)?; // 20 bytes
160-
writer.write_all(&self.gas_used.to_le_bytes())?; // 8 bytes
161-
writer.write_all(&self.gas_limit.to_le_bytes())?; // 8 bytes
162-
writer.write_all(&self.difficulty)?; // 32 bytes
163-
writer.write_all(&self.base_fee_per_gas.to_le_bytes())?; // 8 bytes
155+
writer.write_all(&self.hash)?; // 32 bytes
156+
writer.write_all(&self.parent_hash)?; // 32 bytes
157+
writer.write_all(&self.number.to_le_bytes())?; // 8 bytes
158+
writer.write_all(&self.timestamp.to_le_bytes())?; // 8 bytes
159+
writer.write_all(&self.author)?; // 20 bytes
160+
writer.write_all(&self.gas_used.to_le_bytes())?; // 8 bytes
161+
writer.write_all(&self.gas_limit.to_le_bytes())?; // 8 bytes
162+
writer.write_all(&self.difficulty)?; // 32 bytes
163+
writer.write_all(&self.base_fee_per_gas.to_le_bytes())?; // 8 bytes
164164
Ok(())
165165
}
166166
}
@@ -193,7 +193,8 @@ mod tests {
193193

194194
// Check topics count
195195
let topics_offset = 20 + 32 + 24;
196-
let topics_count = u32::from_le_bytes(bytes[topics_offset..topics_offset+4].try_into().unwrap());
196+
let topics_count =
197+
u32::from_le_bytes(bytes[topics_offset..topics_offset + 4].try_into().unwrap());
197198
assert_eq!(topics_count, 2);
198199
}
199200

runtime/wasm/src/rust_abi/types.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,8 +211,7 @@ impl ToRustWasm for BigDecimal {
211211
impl FromRustWasm for BigDecimal {
212212
fn read_from<R: Read>(reader: &mut R) -> io::Result<Self> {
213213
let s = String::read_from(reader)?;
214-
BigDecimal::from_str(&s)
215-
.map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e))
214+
BigDecimal::from_str(&s).map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e))
216215
}
217216
}
218217

0 commit comments

Comments
 (0)