Skip to content

Commit ac0fe88

Browse files
authored
fix(avm): capping rd size and fixing pipes (#19420)
Returndata size is uncapped in TS but we have an implicit cap in CPP. This makes the cap explicit in the cpp fuzzer and truncates the returndata from TS so that the comparisons do not trivially fail. There's also some clean up of how reading and writing between the fuzzer and TS process works
1 parent 6774da4 commit ac0fe88

File tree

4 files changed

+56
-12
lines changed

4 files changed

+56
-12
lines changed

barretenberg/cpp/src/barretenberg/avm_fuzzer/common/process.cpp

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,21 +49,38 @@ Process::~Process()
4949
void Process::write_line(const std::string& line) const
5050
{
5151
std::string command = line + "\n";
52-
write(stdin_fd, command.c_str(), command.size());
53-
fsync(stdin_fd);
52+
const char* data = command.c_str();
53+
size_t remaining = command.size();
54+
55+
// We use a loop to ensure all data is written but throw if we encounter an error.
56+
// This enables partial writes to be handled correctly.
57+
while (remaining > 0) {
58+
ssize_t written = write(stdin_fd, data, remaining);
59+
if (written < 0) {
60+
if (errno == EINTR) {
61+
continue;
62+
}
63+
throw std::runtime_error("write() error: " + std::string(std::strerror(errno)));
64+
}
65+
data += written;
66+
remaining -= static_cast<size_t>(written);
67+
}
5468
}
5569

5670
std::string Process::read_line() const
5771
{
5872
char buffer[4096]; // NOLINT
5973
std::string response;
6074
ssize_t bytes_read = 0;
61-
fsync(stdout_fd);
6275
while ((bytes_read = read(stdout_fd, buffer, sizeof(buffer))) > 0) {
63-
response.append(buffer, static_cast<size_t>(bytes_read));
64-
if (response.find('\n') != std::string::npos) {
76+
// Check for newline in just the newly read data instead of going back through the entire response
77+
const char* newline_pos = static_cast<const char*>(memchr(buffer, '\n', static_cast<size_t>(bytes_read)));
78+
if (newline_pos != nullptr) {
79+
// Found newline - append only up to and including the newline
80+
response.append(buffer, static_cast<size_t>(newline_pos - buffer + 1));
6581
break;
6682
}
83+
response.append(buffer, static_cast<size_t>(bytes_read));
6784
}
6885
if (bytes_read < 0 && errno != EINTR) {
6986
throw std::runtime_error("read() error: " + std::string(std::strerror(errno)));

barretenberg/cpp/src/barretenberg/avm_fuzzer/fuzz_lib/simulator.cpp

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ using namespace bb::avm2::simulation;
3131
using namespace bb::avm2::fuzzer;
3232
using namespace bb::world_state;
3333

34-
// Helper function to serialize simulation request via
34+
const auto MAX_RETURN_DATA_SIZE_IN_FIELDS = 1024;
35+
36+
// Helper function to serialize simulation request via msgpack
3537
std::string serialize_simulation_request(const Tx& tx,
3638
const GlobalVariables& globals,
3739
const FuzzerContractDB& contract_db)
@@ -79,6 +81,9 @@ SimulatorResult CppSimulator::simulate(fuzzer::FuzzerWorldStateManager& ws_mgr,
7981
.skip_fee_enforcement = false,
8082
.collect_call_metadata = true,
8183
.collect_public_inputs = true,
84+
.collection_limits = {
85+
.max_returndata_size_in_fields = MAX_RETURN_DATA_SIZE_IN_FIELDS,
86+
},
8287
};
8388

8489
ProtocolContracts protocol_contracts{};
@@ -164,8 +169,17 @@ SimulatorResult JsSimulator::simulate([[maybe_unused]] fuzzer::FuzzerWorldStateM
164169
return result;
165170
}
166171

167-
bool compare_simulator_results(const SimulatorResult& result1, const SimulatorResult& result2)
172+
bool compare_simulator_results(SimulatorResult& result1, SimulatorResult& result2)
168173
{
174+
// Since the simulator results are interchangeable between TS and C++, we limit the return data size for comparison
175+
// todo(ilyas): we ideally specfify one param as the TS result and truncate only that one
176+
if (result1.output.size() > MAX_RETURN_DATA_SIZE_IN_FIELDS) {
177+
result1.output.resize(MAX_RETURN_DATA_SIZE_IN_FIELDS);
178+
}
179+
if (result2.output.size() > MAX_RETURN_DATA_SIZE_IN_FIELDS) {
180+
result2.output.resize(MAX_RETURN_DATA_SIZE_IN_FIELDS);
181+
}
182+
169183
return result1.reverted == result2.reverted && result1.output == result2.output &&
170184
result1.end_tree_snapshots == result2.end_tree_snapshots;
171185
}

barretenberg/cpp/src/barretenberg/avm_fuzzer/fuzz_lib/simulator.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,6 @@ Tx create_default_tx(const AztecAddress& contract_address,
9191
bool is_static_call,
9292
const Gas& gas_limit);
9393

94-
bool compare_simulator_results(const SimulatorResult& result1, const SimulatorResult& result2);
94+
bool compare_simulator_results(SimulatorResult& result1, SimulatorResult& result2);
9595

9696
GlobalVariables create_default_globals();

yarn-project/simulator/src/public/fuzzing/avm_simulator_bin.ts

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,23 @@ import {
99
import { GlobalVariables, TreeSnapshots } from '@aztec/stdlib/tx';
1010
import { NativeWorldStateService } from '@aztec/world-state';
1111

12-
import { writeSync } from 'fs';
1312
import { createInterface } from 'readline';
1413

1514
import { AvmFuzzerSimulator, FuzzerSimulationRequest } from './avm_fuzzer_simulator.js';
1615

16+
/** Write data to stdout, letting Node handle buffering. */
17+
function writeOutput(data: string): Promise<void> {
18+
return new Promise<void>((resolve, reject) => {
19+
process.stdout.write(data, err => {
20+
if (err) {
21+
reject(err);
22+
} else {
23+
resolve();
24+
}
25+
});
26+
});
27+
}
28+
1729
// This cache holds opened world states to avoid reopening them for each invocation.
1830
// It's a map so that in the future we could support multiple world states (if we had multiple fuzzers).
1931
const worldStateCache = new Map<string, NativeWorldStateService>();
@@ -96,16 +108,17 @@ async function execute(base64Line: string): Promise<void> {
96108
revertReason: result.revertReason ?? '',
97109
endTreeSnapshots: result.publicInputs.endTreeSnapshots,
98110
});
99-
writeSync(process.stdout.fd, resultBuffer.toString('base64') + '\n');
111+
const base64Response = resultBuffer.toString('base64') + '\n';
112+
await writeOutput(base64Response);
100113
} catch (error: any) {
101114
// If we error, treat as reverted
102115
const errorResult = serializeWithMessagePack({
103116
reverted: true,
104-
output: [] as string[],
117+
output: [] as Fr[],
105118
revertReason: `Unexpected Error ${error.message}`,
106119
endTreeSnapshots: TreeSnapshots.empty(),
107120
});
108-
writeSync(process.stdout.fd, errorResult.toString('base64') + '\n');
121+
await writeOutput(errorResult.toString('base64') + '\n');
109122
}
110123
}
111124

0 commit comments

Comments
 (0)