Skip to content

Execute sc does not return execution result #355

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
modship wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Execute sc does not return execution result #355

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
914aca6
remove "main" condition
modship Jan 6, 2026
05426ca
Fix bad offset
modship Jan 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 9 additions & 18 deletions src/as_execution/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,26 +104,17 @@ impl ASContext {

match res {
Ok(value) => {
if function.eq(crate::settings::MAIN) {
let remaining_gas = if cfg!(feature = "gas_calibration") {
Ok(0_u64)
} else {
get_remaining_points(&self.env, store)
};

return Ok(Response {
ret: Vec::new(), // main return empty vec
remaining_gas: remaining_gas?,
init_gas_cost: 0,
#[cfg(feature = "execution-trace")]
trace: Default::default(),
});
}
let ret = if let Some(offset) = value.first() {
if let Some(offset) = offset.i32() {
let buffer_ptr = BufferPtr::new(offset as u32);
let memory = instance.exports.get_memory("memory")?;
buffer_ptr.read(memory, store)?
// Offset 0 means no return value in AssemblyScript
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment claims "Offset 0 means no return value in AssemblyScript" but there is no documentation or reference to support this convention in the codebase. This assumption should be validated against AssemblyScript's FFI documentation or the massa-sc-std implementation. If this is indeed the convention, consider adding a reference to the documentation. If not, this logic may incorrectly treat valid return values as empty.

Suggested change
// Offset 0 means no return value in AssemblyScript
// AssemblyScript uses 0 as the null reference / null pointer value, and
// the massa-sc-std ABI follows this convention for optional return buffers:
// a returned pointer of 0 means "no return value" (i.e. no buffer to read).
// See AssemblyScript's memory model and pointer semantics:
// https://www.assemblyscript.org/runtime.html#memory

Copilot uses AI. Check for mistakes.
if offset == 0 {
Vec::new()
} else {
let buffer_ptr = BufferPtr::new(offset as u32);
let memory = instance.exports.get_memory("memory")?;
// If reading fails (e.g., invalid offset), return empty vec
buffer_ptr.read(memory, store).unwrap_or_default()
Comment on lines +115 to +116
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using unwrap_or_default() here silently swallows errors that occur when reading memory with an invalid offset. This masks legitimate bugs where a function returns a corrupted or invalid offset (neither 0 nor a valid memory location). Instead, the error should be propagated to the caller or logged so that invalid memory access issues can be properly diagnosed. Consider using the ? operator to propagate the error instead.

Suggested change
// If reading fails (e.g., invalid offset), return empty vec
buffer_ptr.read(memory, store).unwrap_or_default()
buffer_ptr.read(memory, store)?

Copilot uses AI. Check for mistakes.
}
Comment on lines 107 to +117
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of the special case for the main function changes the established behavior. Previously, the main function always returned an empty vector. Now it can return values based on the offset. This breaks the test at line 1202-1204 in src/tests/tests_runtime.rs which explicitly expects main to return an empty vector with the comment "Note: for now, exec main always return an empty vec". Either update the test to reflect the new behavior, or ensure backward compatibility by handling the main function case appropriately.

Copilot uses AI. Check for mistakes.
} else {
vm_bail!("Execution wasn't in capacity to read the return value")
}
Expand Down
Loading