[rosetta] Audit, test, and restructure aptos-rosetta#20192
Open
gregnazario wants to merge 3 commits into
Open
Conversation
Adds a durable audit, a full-coverage characterization suite, and a
legibility rewrite of the aptos-rosetta sidecar. Public server behavior
(routes, wire JSON, error codes) is unchanged except for three documented
fixes, each pinned by a golden test.
Audit (docs/):
- SPEC_DEVIATIONS.md: complete inventory of intentional, preserved
deviations from the Rosetta/Mesh spec (fake block hashes, sub-account
sentinels, custom metadata bags, mandatory construction metadata, Y2K
timestamp clamp, InternalOperation smuggling, 36 HTTP-500 error codes,
no-0x-prefix hashes, ...).
- BEHAVIOR_CHANGES.md: changelog of every intentional change (BC-1..BC-7).
- COVERAGE.md: coverage baseline and deferred-coverage rationale.
Tests:
- New object-safe NodeClient seam (node_client.rs) over aptos_rest_client,
with a production RestNodeClient impl and mockall-based mocking.
- 41 characterization tests: offline endpoints via warp routes, online
handlers via a mocked NodeClient, the full 36-code error table, an
offline construction round-trip, and wiremock tests over the real
client's HTTP wire path.
Rewrite (behavior-preserving unless noted):
- error.rs: single ApiError::info() source-of-truth table.
- types/objects.rs (3034 lines) split into objects/{currency,operation,
transaction,internal_op}.rs behind a re-export shim.
- construction.rs (1561 lines) split into per-endpoint submodules plus
shared routes/helpers behind a re-export shim.
- common.rs: doc comments citing SPEC_DEVIATIONS; simplified get_timestamp.
Documented wire changes (with golden tests):
- BC-1: fix typo'd error messages (codes unchanged).
- BC-2: map AptosErrorCode::InvalidTransactionUpdate to code 29 (was 28).
- BC-6: advertise update_commission in network/options (was omitted).
Audit of the rosetta-cli conformance assets (rosetta_cli.json, aptos.ros): - README stated the fake block hash is `<chain_id>:<block_height>` (colon), but the code produces (and BlockHash::from_str only accepts) a hyphen: `<chain_id>-<block_height>`. A caller building a block identifier by hash with a colon would hit InvalidInput. Corrected to match the code and SPEC_DEVIATIONS.md §2. - Document the conformance assets and their load-bearing assumptions in SPEC_DEVIATIONS.md §15 (network name "TESTING", APT move_type-only currency shape, hyphen block-hash separator, supported ops), and note that conformance is a manual step whose automated equivalent is the smoke-test rosetta suite.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
wiremock 0.6.5 raised its `futures` floor to ^0.3.31, which the pinned aptos-labs/futures-rs backport fork (0.3.30) cannot satisfy. That pulled a second, upstream futures 0.3.32 into the graph, bypassing the fork patch and flipping consensus onto a futures-channel whose `UnboundedReceiver::try_next` is `#[deprecated]` — failing `cargo xclippy` (`-D warnings`) across consensus. Pin the dev-dependency to =0.6.4 (identical wire.rs test API, only requires futures ^0.3.5) so the fork patch applies again. The lockfile now matches origin/main except for the wiremock subtree; futures* and hyper return to their fork/main versions.
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.
Summary
Audits, tests, and restructures the
aptos-rosettasidecar for legibility. Public server behavior (routes, wire JSON, error codes) is unchanged except for three documented fixes, each pinned by a golden test and now confirmed by the full e2e suite.Audit (
crates/aptos-rosetta/docs/)SPEC_DEVIATIONS.md— complete inventory of intentional, preserved deviations from the Rosetta/Mesh spec that were previously undocumented but load-bearing (fake<chain_id>-<height>block hashes, sub-account sentinels, custom metadata bags, mandatory construction metadata, Y2K timestamp clamp,InternalOperationsmuggling, 36 HTTP-500 error codes, no-0x-prefix hashes, rosetta-cli conformance-asset contract, …).BEHAVIOR_CHANGES.md— changelog of every intentional change (BC-1..BC-7) with before/after + rationale.COVERAGE.md— coverage baseline and deferred-coverage rationale.Tests
NodeClientseam (node_client.rs) overaptos_rest_client::Client, with a productionRestNodeClientimpl andmockall-based mocking — enables offline handler unit tests.NodeClient, the full 36-code error table, an offline construction round-trip (preprocess→payloads→parse→combine→hash), andwiremocktests over the real client's HTTP wire path.Rewrite (behavior-preserving unless noted)
error.rs: singleApiError::info()source-of-truth table (was four disagreeing parallel matches).types/objects.rs(3034 lines) →objects/{currency,operation,transaction,internal_op}.rsbehind a re-export shim.construction.rs(1561 lines) → per-endpoint submodules + sharedroutes/helpersbehind a re-export shim.common.rs: doc comments citingSPEC_DEVIATIONS.md; simplifiedget_timestamp.All module moves are verbatim — no logic, signature, or serde changes; every external
crate::types::*/crate::construction::*path resolves as before.Documented wire changes (each with a golden test)
AptosErrorCode::InvalidTransactionUpdateto code 29 (was incorrectly 28).update_commissioninnetwork/options(was omitted fromOperationType::all()).Also fixes a README bug: the fake block hash separator is
-, not:(the code only accepts-).Test plan
cargo test -p aptos-rosetta→ 41 passedcargo check(lib + cli + all-targets),cargo +nightly fmt --check,cargo sort,cargo machete→ clean; crate clippy clean (only repo--A-allowed lints)RUST_MIN_STACK=33554432 cargo test -p smoke-test rosetta:: -- --test-threads=1→ 7 passed; 0 failed (account balance, block, block transactions, delegation pool, invalid/failed txn gas, network, transfer) against a real LocalSwarmNot covered here
rosetta-cli check:data/check:construction— manual step needing the external binary + an online/offline node pair (conformance assets audited & consistent; seeSPEC_DEVIATIONS.md§15).🤖 Generated with Claude Code