Fixes for the Embers demo#175
Open
dylon wants to merge 19 commits into
Open
Conversation
…r overrides - Migrate firefly-client from protobuf codegen to direct gRPC tonic client, removing all local .proto files (CasperMessage, DeployService, RhoTypes, etc.) and the build.rs protobuf compilation step - Add per-bind peek field to ReceiveBind in Rholang templates (agents, agents_teams, oslfs) to match updated f1r3node protocol - Add pre-deploy diagnostic probes in agents_teams deploy flow: registry entry probe, system URI probe, and persistent handler probe to detect trie state issues before deploying - Add explore-deploy retry logic with exponential backoff and detailed response logging in read_node_client (block info, empty expr warnings, return channel diagnostics) - Restructure node_events WebSocket handler with deploy finalization tracking and error/cost reporting per deploy - Add docker-compose.override.yaml with local f1r3node image and comprehensive diagnostic tracing targets (cost_trace, state_hash, channel_trace, comm_side, replay_diag) - Update docker-compose service configurations for multi-network staging support - Add rust-toolchain.toml for consistent toolchain version - Bump dependencies in Cargo.lock
…ayer Add 151 new tests (37 -> 188 total) covering the firefly-client gRPC migration, retry logic, event handling, and all domain service deploy flows that were untested in the prior commit. Infrastructure: - Extract ReadNode, WriteNode, NodeEventSource traits for testability - Make all 5 domain services generic with default type params - Add MockReadNode, MockWriteNode, MockNodeEventSource test doubles - Add NodeEvents::new_for_test() for event injection without WebSocket - Add wiremock, tokio test-util, tracing-test dev-dependencies firefly-client tests (90 new): - models.rs: WalletAddress/Uri validation, ReadNodeExpr conversion, Either deserialization, NodeEvent parsing, DeployData builder - read_node_client.rs: HTTP mock tests for get_data, retry, errors - write_node_client.rs: deploy ID parsing, builder defaults - node_events.rs: subscription, dispatch, cache, race-free pattern - errors.rs: Display verification for all error variants embers tests (25 new): - common.rs: prepare_for_signing, AES encrypt/decrypt, PositiveNonZero - agents_teams/deploy.rs: deploy_signed with/without system contract - agents/deploy.rs: deploy_signed both + main only - agents_teams/run_agents_team.rs: chain error, not finalized - testnet/deploy_test.rs: prepare, env/test failure, success with logs - Template rendering for agents, agents_teams, wallets init templates
The rust/dev merge changed defaults.conf to disable heartbeat and raise fault-tolerance-threshold to 0.67, causing standalone validator nodes to never propose blocks after genesis. Add explicit --heartbeat-enabled and --fault-tolerance-threshold=0.0 flags to both mainnet and testnet validator services in docker-compose.yaml. Also add deploy ID logging after mainnet and testnet bootstrap submissions to aid startup diagnostics.
The CI cannot resolve the local path `../../../f1r3node/models`. Point the f1r3node-models dependency at the f1r3node GitHub repo (rust/dev branch) so CI can fetch it.
Upgrade aws-lc-sys 0.37.1 -> 0.39.1 (5 CVEs), quinn-proto 0.11.13 -> 0.11.14 (DoS), and rustls-webpki 0.103.9 -> 0.103.10 (CRL bypass). Ignore RUSTSEC-2025-0141 (bincode unmaintained) in audit config since it is a transitive dependency from f1r3node and cannot be resolved in embers.
The project uses edition 2024 and nightly features (smallvec may_dangle). Remove the explicit `toolchain: nightly, stable` from the CI setup action so the runner uses the nightly-2026-02-09 specified in rust-toolchain.toml.
The rust-toolchain.toml already sets nightly-2026-02-09 as the default. The explicit `toolchain = "nightly"` in cargo-make tasks fails in CI because only the dated nightly is installed, not an alias named "nightly".
The f1r3node transitive dependency gxhash requires AES and SSE2 intrinsics at compile time. Set RUSTFLAGS in the CI build job to enable them on the ubuntu-latest runner.
.pgmcp.toml configures a local code-indexing tool and describes an individual developer's environment rather than the project, so it should not be tracked. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Point the models crate at F1R3FLY-io/f1r3node-rust, pinned to an explicit
rev rather than tracking a moving branch, and teach the read-node model the
two response shapes the newer node emits:
* ExprBundle. A registry lookup (rho:registry:lookup) returns the
unforgeable name inside a bundle+{...} permission wrapper. With no
matching variant this failed to deserialize and blocked startup with
"failed to deserialize intermediate model". The wrapper carries nothing
a JSON consumer needs, so it resolves to its inner value, mirroring the
way ExprUnforg unwraps to its data.
* NodeEvent::Other. The node emits lifecycle/status events embers does not
act on (node-started, sent-approved-block, transfers-available, ...).
Each previously failed to deserialize and logged a spurious warning on
every tick; a #[serde(other)] catch-all absorbs them so the
deploy-tracking stream is left undisturbed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Neither node client had any transport bounds, and both could wedge until the process was restarted. Read (HTTP/explore-deploy): reqwest's default client sets no request timeout at all, so get_data and get_data_or_none were entirely unbounded -- including the periodic registry health check, which calls get_data directly. reqwest's default tcp_user_timeout (30s) does not cover this: it fires only when data goes unacked, so a node that accepts the connection and ACKs but then stalls at the HTTP layer hangs the call forever. Add a ReadNodeConfig with a 20s request timeout (~80x the observed healthy 0.24s latency, and small enough that two attempts still fit inside the 45s cap used by get_data_with_retry), a 5s connect timeout, and TCP keepalive. Write (gRPC/deploy service): a default tonic Endpoint sets none of these, which made the client wedge permanently. With no HTTP/2 keep-alive a half-open connection (NAT eviction, container blip, peer restart without a clean FIN) is never detected, so the h2 connection never reports an error and tonic's Reconnect never re-establishes; with no deadline, calls on that dead connection hang forever. Because every Channel clone shares one tower Buffer worker, a single hung call parks the worker and every caller stalls -- exactly the observed failure, where all */prepare endpoints hung indefinitely while HTTP reads stayed fast. Endpoint::timeout alone is not sufficient, for two independent reasons: tonic's GrpcTimeout layer sits inside the tower Buffer, so its timer only starts once the worker dispatches the request (a request queued behind a stuck call has no timer at all), and GrpcTimeout resolves as soon as response headers arrive, leaving a streaming body unbounded -- get_head_block_index reads a server-streaming body. So each call is also wrapped in an outer tokio timeout that bounds queue-wait, connect, handshake, headers and body. Keep-alive is deliberately generous (30s interval, 30s ack window): unlike TCP keepalives, an h2 PING must be acked by the node's application task, which can stall for seconds under block-processing load. Measured against a live f1r3node, 10s/5s produced spurious KeepAliveTimedOut errors on a healthy connection. Idle pings stay off because grpc-java/Netty servers default to permitKeepAliveWithoutCalls=false and answer them with GOAWAY(ENHANCE_YOUR_CALM); the idle case is covered by tcp_keepalive instead, which is invisible to gRPC. The tokio "net" dev-feature backs the new test that binds a listener which accepts TCP but never speaks HTTP/2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A client whose WebSocket was down at finalization -- or that had not yet subscribed when its deploy landed -- never learned the deploy had finalized, and reported a successful deploy as a timeout. Retain each finalized deploy in a cache for FINALIZED_CACHE_TTL and replay it to late subscribers. The previous (errored, Instant) tuple could not support this: it lacked the cost and the deployer, so a wallet-scoped DeployEvent could not be reconstructed from it. FinalizedDeploy now carries everything the event needs. The ordering between subscribe and dispatch is load-bearing. subscribe joins the live broadcast (S1) before scanning the cache (S2); dispatch inserts into the cache (D1) before sending to the broadcast (D2). For any deploy D: if D2 happens after S1, our receiver's cursor was set at S1 and we get it live; if D2 happens before S1, then D1 < D2 < S1 < S2 and the S2 scan must observe it. The cases are exhaustive, so no deploy can fall between the two paths -- whereas the naive order (scan, then subscribe) leaves exactly that gap, which is the bug being fixed. Both paths can fire for the same deploy, so replayed ids are deduped against the live stream in WalletSubscription::poll_next. Replay is capped at MAX_REPLAY_EVENTS so a client cannot be flooded on connect. Also fix a broadcast Lagged arm that returned without re-polling, leaving the task with no waker registered and stalling the stream forever, and hoist dispatch into a single dispatch_events used by both NodeEvents::new and new_for_test -- the two held copy-pasted, already-divergent copies, so tests could exercise different logic than production. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The rholang-rs cost-accounting-transpiler branch makes `agent` a
process-initial soft keyword: the grammar carries agent_block as an
alternative of _proc, together with keyword extraction (word: $ => $.var)
and no conflicts declaration. The lexer therefore emits the 'agent' keyword
token -- never `var` -- in every state that can start a _proc, so
`agent.union(...)`, `@(version, agent)` and friends fail to parse with
Unexpected('.') / UnexpectedVar.
`agent` is an ordinary bound variable at these three sites, so renaming it
sidesteps the collision entirely. Keyword extraction matches the exact
lexeme only, which is why the neighbouring agentVersions and
agentLastVersion already parsed fine.
The f1r3node-rust rev embers pins (91b5c70a) still ships an older parser
with no agent_block, so this template parsed there either way; the rename is
what lets the same template also run against a transpiler-branch node.
Verified with tree-sitter parse against the transpiler grammar: the fully
rendered agents env parses clean, and embers boots against that node with
every env registering.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two template bugs, both of which made wallets silently fail rather than error. Vault URN: f1r3node migrated the REV vault registry from the legacy rho:rchain:revVault to rho:vault:system (SystemVault). The legacy URN still resolves, which is what makes this so quiet -- but it is a separate, empty vault. Transfers into it never touch the SystemVault balances that deploy pre-charge reads, so funded wallets stayed broke. Point the wallets and testnet-funding templates at the vault the node actually charges against. Registry peek race: each shared resource was looked up with rho:registry:lookup onto the very channel the log contracts later peek with `<<-`. The lookup result is a (nonce, resource) tuple, and a bare peek can bind that tuple instead of the unwrapped resource, so the join never fires -- transfers do not credit and balance queries return nothing. Look each resource up on its own channel and forward only the unwrapped resource onto the peeked channel, which removes the ambiguity. Applied to treeHashMap, revVault, either and stack in wallets/init.rho and to treeHashMap and stack in testnet/init.rho. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The f1r3node tuplespace stores Rholang string literals verbatim -- the
parser does not process escape sequences (verified: ch!("a\"b") reads back
as a\"b). User strings that embers embedded into Rholang source via
escape_rho_string (\ -> \\, " -> \") therefore come back still escaped, so
an agent whose code contains @nil!("foo") reads back as @nil!(\"foo\").
The Graph deserializer already undid this inline. Hoist it into a shared
unescape_rho_string plus a deserialize_unescaped_opt serde helper, and apply
it to the other user-authored string fields read back out of the tuplespace:
Agent::code and Oslf::query.
For Agent::code this is more than a display concern -- it is the form the
agent-deploy path re-embeds as a Rholang term, so leaving it escaped
compounds the escaping on every round trip.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two causes of the same symptom -- a testnet deploy reporting no logs. Read-after-write lag: once the observer's WebSocket reports a deploy finalized, there is still a brief window before that block's RSpace state is queryable via explore-deploy on the observer node, so the single-shot log read could intermittently miss logs the deploy had written. The window widens under load. Retry on empty, mirroring the analogous deploy-then-read retry in run_agents_team.rs. A genuinely empty deploy (a `Nil` test that logs nothing) simply exhausts the retries and resolves to Ok(None) -> logs: [], which also fixes it surfacing as a 500 via ReturnValueMissing; the worst-case retry window stays well under the client's 45s cap, so this cannot become a timeout. Silent env failure: the testnet env was never verified readable after its init deploy, unlike the mainnet envs. An unfunded testnet service wallet makes that init deploy finalize as a no-op, and the failure then surfaced only later, as empty testnet deploy logs. Verify it at bootstrap so it fails where it happens. MockReadNode previously returned immediately from get_data_with_retry and so could not have caught either bug. It now models the real client faithfully -- retrying only on ReturnValueMissing, up to max_retries, with the delay ignored to keep tests fast -- and gains on_code_containing_empty_then to sequence empty-then-data. The two new tests cover both paths: logs that only become visible on the third read, and a no-log deploy that must still resolve to an empty list. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
compile_proto.py still pointed at packages/firefly-client/protobuf, the vendored proto copy that 43d5457 deleted when firefly-client moved onto the f1r3node models crate. The script had been dead ever since, and silently so: an empty rglob just printed "No .proto files found." and exited 0. Compile from the f1r3node-rust `models` subcrate instead -- the same single source of truth firefly-client builds against -- located via F1R3NODE_MODELS_DIR and defaulting to a cost-accounting-transpiler worktree beside this repository. Vendoring is what let the protos drift in the first place. Missing uv or missing protos now fail with an actionable message rather than a no-op exit 0. Only the client-facing protos are compiled, mirroring firefly-client's `pub use f1r3node_models::{casper, rhoapi, servicemodelapi}` plus routing. The internal node protos (RholangScalaRustTypes, RSpacePlusPlusTypes) are not part of the client API surface, and google/protobuf/compiler's plugin.proto was only ever generated because the old rglob swept the entire vendored tree, hence its removal here. The google well-known types ship with grpcio-tools and resolve automatically. All protos now go through a single protoc invocation: several files share the casper / casper.v1 packages, and betterproto2 only emits coherent package modules when it sees them together. The betterproto2 compiler plugin pins an older ruff than this project uses, so it is deliberately not a project dependency. It is build-time-only -- the tests import only the betterproto2 runtime -- and is supplied from an isolated, ephemeral uv environment pinned to that runtime's minor line. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tests/client.py imports `from Crypto.Hash import keccak`, but pycryptodome was never declared, so the import only resolved when the package happened to be present in the environment. Declare it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes:
build.rs protobuf compilation step in favor of the f1r3node-models crate
protocol
deploying
diagnostics) in ReadNodeClient
per-deploy oneshot notification channels
explicit propose calls, with 60-second timeout and deploy ID logging
defaults.conf change that disabled auto-propose
MockReadNode/MockWriteNode/MockNodeEventSource test doubles
AES encrypt/decrypt, PositiveNonZero, domain deploy flows, and template rendering