Repository navigation
[F40] Unbounded query_state Batch Processing Can Cause Execution Lock Contention and Node DoS #5057
Description
⚠️ Warning — This finding was routed to an issue rather than a PR because it changes a JSON-RPC / gRPC interface or its observable behavior (accepted inputs, returned results, error-vs-silent-drop, or transport/security posture). Per maintainer guidance this is treated as breaking, because it can break external tooling and clients; the gRPC proto types are additionally auto-generated (externalmassa_proto_rs) and must not be hand-edited. It needs an interface-compatibility decision, not a drop-in PR.
Source
AI security-scan Finding 40 (severity: major). Tracked in SUBMISSIONS.md as F40.
Affected file: massa-execution-worker/src/controller.rs
Summary
The query_state logic takes a read lock on self.execution_state.read() before it starts processing a batch. It keeps that lock for the full batch, including expensive query items and serial evaluation of all responses. Because the execution worker needs self.execution_state.write() to process final slots, candidate slots, and queued read-only executions, a long or heavy query batch can block execution progress for the whole node. A remote caller can abuse the public API batch limit of up to 128 items to slow the node, make it fall behind, or make it unresponsive.
Root cause
The main root cause is that query_state acquires self.execution_state.read() too early and holds it for too long. The lock remains held until every request item has been evaluated and appended to the response, instead of being limited to the small part that actually needs shared access.
A second root cause is that the execution worker depends on self.execution_state.write() to execute final slots, candidate slots, and queued read-only executions. Since the read lock from query_state is held across the full batch, this write access cannot proceed while the batch is still running.
A third root cause is that public API wrappers still allow batches of up to 128 query items, and expensive items such as Events, deferred-credit lookups, and repeated datastore scans are executed serially under the same read lock. This increases the time that self.execution_state.read() stays held and makes lock contention externally triggerable.
Affected code
/// Atomically query the execution state with multiple requests
fn query_state(&self, req: ExecutionQueryRequest) -> ExecutionQueryResponse {
let execution_lock = self.execution_state.read();
let mut resp: ExecutionQueryResponse = ExecutionQueryResponse {
responses: Vec::with_capacity(req.requests.len()),
candidate_cursor: execution_lock.active_cursor,
final_cursor: execution_lock.final_cursor,
final_state_fingerprint: execution_lock.get_final_state_fingerprint(),
};
for req_item in req.requests {
let resp_item = match req_item {
ExecutionQueryRequestItem::AddressExistsCandidate(addr) => {
Ok(ExecutionQueryResponseItem::Boolean(
execution_lock
.get_final_and_candidate_balance(&addr)
.1
.is_some(),
))
}
ExecutionQueryRequestItem::AddressExistsFinal(addr) => {
Ok(ExecutionQueryResponseItem::Boolean(
execution_lock
.get_final_and_candidate_balance(&addr)
.0
.is_some(),
))
}
ExecutionQueryRequestItem::AddressBalanceCandidate(addr) => {
let (_final_v, speculative_v) =
execution_lock.get_final_and_candidate_balance(&addr);
match speculative_v {
Some(balance) => Ok(ExecutionQueryResponseItem::Amount(balance)),
None => Err(ExecutionQueryError::NotFound(format!("Account {}", addr))),
}
}
ExecutionQueryRequestItem::AddressBalanceFinal(addr) => {
let (final_v, _speculative_v) =
execution_lock.get_final_and_candidate_balance(&addr);
match final_v {
Some(balance) => Ok(ExecutionQueryResponseItem::Amount(balance)),
None => Err(ExecutionQueryError::NotFound(format!("Account {}", addr))),
}
}
ExecutionQueryRequestItem::AddressBytecodeCandidate(addr) => {
let (_final_v, speculative_v) =
// … (truncated)Recommendation
Reduce the time self.execution_state.read() is held during query_state processing. Acquire the read lock only around the minimal state access needed for each individual query item, and release it before performing expensive computation, response assembly, or any work that does not require shared access to the execution state.
Avoid processing large batches under a single long-lived lock. Handle items in smaller units so the execution worker can regularly obtain self.execution_state.write() for final slots, candidate slots, and queued read-only executions.
Add stricter limits to publicly accessible query_state batches, especially for expensive request types such as Events, deferred-credit lookups, and repeated datastore scans. Consider separate per-item cost controls, lower batch caps, request budgeting, and rate limiting to prevent remote users from triggering prolonged lock contention.
If certain queries are inherently expensive, move them to snapshots, cached views, or other mechanisms that do not require holding the live execution-state lock for the full duration of evaluation.
Why P0 / breaking
This change affects consensus, wire/serialization format, signatures, economics, block/tx validity, or otherwise requires a coordinated (versioned / hard-fork) rollout. It is therefore filed as an issue for design discussion rather than a direct PR. See SUBMISSIONS.md Part D.
Activity
modship commentedon Aug 12, 2026
The core difficulty here is that
query_statereturns a single(candidate_cursor, final_cursor, final_state_fingerprint)for the whole batch. Clients treat that as “all responses were computed at this execution state.”If we release
execution_state.read()between items so writers can progress, later items can observe a newer state than the cursors captured at the start. Responses would then no longer match the returned cursors.We can fix it if we assume that.
damip commentedon Sep 2, 2026
@modship is right, and it's more than a detail:
query_stateIS an atomic batch. The cursors + fingerprint in the response are the contract, clients rely on them to know that all the items describe the same slot (balance + N datastore entries of a contract in one call, bridge, indexers...). Releasing the lock between items would make the items mutually inconsistent while still stamped with one cursor. Silent read corruption is much worse than the DoS this is supposed to mitigate. The recommendation of this issue is rejected. Never split the lock.Rule to apply everywhere: bound the WORK per batch, never the lock.
parking_lot::RwLockis fair (a waiting writer blocks new readers), so execution waits at most for the heaviest batch in flight. Bounding the heaviest batch is the whole fix.I checked all the PRs from the scan that touch the execution controller / worker / API read paths and all the batch getters of the controller: none splits a lock, all hold one
execution_statelock per batch. #5236 (F41) is the pattern to follow: per-item error when over budget, lock kept, cursors stay valid.scan_datastore(#5189) takes the final_state / active_history locks separately but always under the outer execution_state lock, so it's fine too.What to do:
query_stateand the other batch getters: responses stamped with cursors/fingerprint must be computed under a singleexecution_statelock, never release it between items, cost is bounded via budgets not by splitting the lock. Otherwise the next generated PR will try this again.Eventsitems xmax_event_per_query=7000 is still 900k events under one lock) and for datastore keys.get_addresses_infos(behindget_addresses): lists EVERY datastore key of every address withcount = None, up to 128 addresses under one lock. Neither [F42] propagate the configured datastore key query cap from RPC/gRPC parsing to the scan #5189 nor [F41] feat: implem cumulative budget for query_state #5236 touches it. Cap it withmax_datastore_keys_queryand put it on the F42 breaking changes list (the docs already steer clients to the paginated endpoint), or remove the key lists fromget_addresses.query_state(config, ~500ms). After the deadline the remaining items return an error, the items already computed are all from the same state so the cursors stay valid. Non-deterministic under load so it's a safety net, not the main bound.query_statereads a consistent view without holding the execution lock at all. Real refactor.Unrelated to the scan but same class, found while checking: JSON-RPC
get_operationscallsget_ops_exec_statusonce thenget_transfer_for_opper op under separate locks, so status and transfers can come from different slots. Minor, to fold into one locked call or document.Keeping this issue open for 1-4.
Leo-Besancon commentedon Sep 4, 2026
Note that this corresponds to the following followup issue: #5255
[F40] refactor(execution-worker): enforce atomic batch reads with Bat…
[F40] feat(execution): batch-wide event budget in query_state (#5255)
[F40] refactor(execution-worker): enforce atomic batch reads with Bat…
[F40] feat(execution): batch-wide event budget in query_state (#5255)
[F40] refactor(execution-worker): enforce atomic batch reads by borro…
[F40] feat(execution): batch-wide event budget in query_state (#5255)
[F40] refactor(execution-worker): enforce atomic batch reads by borro…
[F40] feat(execution): batch-wide event budget in query_state (#5255)
bilboquet commentedon Sep 15, 2026
Tracking the remaining items as agreed with the spec discussion above:
get_addresses_infoshotspot) stays here: interface decision pending (cap vs key-list removal), separate breaking PR to come.This issue stays open until items 3 and 4 land.
[F40] bound query_state batch work: shared event budget, atomic reads (…
[F40] fix(execution): cap datastore keys in get_addresses_infos (#5057 …
[F40] fix(execution): cap datastore keys in get_addresses_infos (#5057 …