Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

kms: performance findings — eight serialized round trips on every CVM boot

Aperta
#1,259 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
5/5
Tempo stimato
Più di una settimana
Idoneità per principianti
32/100
Tipo di issue
Refactoring
Chiarezza
Abbastanza chiara
Stato di attività
Attiva
Stack tecnologico
rust, solidity, typescript

Direzione di ricerca

Treat this as a collection of separate changes rather than one task. Start with main_service.rs:360-369 and auth-eth-bun/index.ts:131-198, then read main_service/upgrade_authority.rs and dstack/http-client/src/lib.rs:90; choose one bounded optimization and trace its existing callers. Done means the selected path removes the stated repeated work without changing authorization or revocation behavior.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

rust

A read of the KMS Rust service, all four auth backends, both contracts, and the verifier/ra-rpc paths the KMS drives, looking for cost rather than correctness. No benchmarks were run — every number below is a complexity class, a counted round trip, or an estimate labelled as such.

Readability and maintainability findings from the same sweep are collected separately; this issue is only the parts that cost time or bandwidth.

Two findings from the sweep turned out to be correctness bugs already fixed in #1247 (the auth-simple empty-device-allowlist fail-open, and the SNP device_id 64-vs-32-byte mismatch), so they are not repeated here.

The headline: ~8 serialized network round trips per GetAppKey

main_service.rs:360-369, auth-eth-bun/index.ts:131-198

get_app_key makes two sequential POSTs to the auth API — ensure_self_allowed() → POST /bootAuth/kms, then ensure_app_boot_allowed() → POST /bootAuth/app. And EthereumBackend.checkBoot makes four sequential chain RPCs each:

const chainId = await this.client.getChainId();      // 1
const head    = await this.client.getBlockNumber();  // 2
response      = await this.client.readContract({ isAppAllowed, blockNumber }); // 3
gatewayAppId  = await this.client.readContract({ gatewayAppId, blockNumber }); // 4

That is 8 strictly serialized Ethereum RPC round trips on every CVM boot, each preceded by a fresh TCP/TLS connect (see below). Against a hosted endpoint at a plausible 80–150 ms RTT that is 0.6–1.2 s of boot latency, all of it avoidable.

Three of the four are unnecessary as written:

  • getChainId() is invariant for a given endpoint — a startup assertion, not a per-decision one.
  • The two readContracts are pinned to the same blockNumber and are independent: await in a loop where Promise.all is correct.
  • viem's HTTP transport supports http(rpcUrl, { batch: true }), which coalesces JSON-RPC calls issued in the same tick into one POST — turning steps 3+4 into a single request.

Fix: assert chainId once at startup, Promise.all the two reads, construct the transport with { batch: true }. 4 sequential round trips → 2. ~15 lines.

Separately, the two is_app_allowed calls in Rust are independent and could run under tokio::try_join! — with the trade-off that the app query would then be issued even when the KMS's own self-authorization is about to fail. Worth stating explicitly if taken; the viem half gets most of the win without it.

A fresh reqwest::Client per authorization decision

main_service/upgrade_authority.rs:98,102

async fn http_get<R: DeserializeOwned>(url: &str) -> Result<R> {
    send_request(reqwest::Client::new().get(url), url).await
}

The workspace builds reqwest with features = ["rustls", "hickory-dns", ...], so each construction builds a rustls ClientConfig and a Hickory resolver (reading /etc/resolv.conf), then discards the connection pool — guaranteeing a fresh TCP connect, plus a full TLS handshake if the auth API is HTTPS, per decision. On a boot critical path.

This is #741's lesson, and the project already has the answer: dstack/http-client/src/lib.rs:90 keeps static POOLED/FRESH: OnceLock<reqwest::Client> with a comment arguing exactly this — and upgrade_authority.rs:11 already imports from that crate. The good pattern and the bad one are in the same file's import list. verifier/src/verification.rs:1295 (download_image) has the same shape. ~10 lines.

Together these two are ~25 lines across two files and remove roughly half the serialized network latency from every CVM boot. If only one thing here gets done, do these.

auth-simple re-reads and re-validates its whole config per request

auth-simple/index.ts:77-95, called from :95 and :224

Per bootAuth request: an existsSync, a synchronous readFileSync (Bun is single-threaded — this blocks the event loop for every other in-flight request), a JSON.parse, and a full zod walk of apps — O(n) in registered apps with a fresh object graph each time.

The re-read is not accidental: it is what makes a config edit take effect immediately, the same property repeated_authorization_is_never_served_from_a_decision_cache pins on the Rust side. So the fix must not be a TTL cache. statSync and reuse the parsed value when mtimeMs and size are unchanged: revocation-on-edit is preserved exactly (an edit changes mtime) and the per-request cost drops to one stat. ~20 lines.

And in the same file, a linear scan of something that is already a map (:173-175):

const appConfig = Object.entries(config.apps).find(([id]) => normalizeHex(id) === appId)?.[1];

Object.entries materializes an array and normalizeHex allocates a string per entry, per request. Alongside it, osImages.map(normalizeHex), composeHashes.map(normalizeHex) and devices.map(normalizeHex) rebuild normalized arrays every call and then .includes() them. Normalizing once at load (behind the memo above) into Map + Set makes all of it O(1) with zero per-request allocation. ~25 lines, folds naturally into the memo.

The vm_config blob: 3 copies and 4 JSON parses per GetAppKey

main_service.rs:285-301,324; verifier/src/verification.rs:793-810; dstack-attest/src/attestation.rs:410-427,932-935

self.verify_os_image_hash(vm_config_str.into(), att)          // copy 1: &str -> String
// then, inside:
let raw_config = if vm_config.is_empty() { attestation.config.clone() }  // copy 2
                 else { vm_config.clone() };                             // copy 3

raw_config is consumed only by the SEV-SNP arm, so on the TDX path — the dominant one — copies 2 and 3 are dead weight.

The parse count is worse: decode_vm_config_with_fallback parses the string twice (once as serde_json::Value, once as VmConfig), and it is invoked twice per request — from decode_app_info_ex and from verify_os_image_hash. Four full JSON parses of the same string per GetAppKey, and for SEV-SNP that string embeds a serialized measurement document, so it is not small.

Fix now: change verify_os_image_hash(vm_config: String) → (&str) and make raw_config a borrow. Kills all three copies, changes no semantics, ~8 lines. The parse-once refactor is bigger and wants its own change.

Concurrent boots of the same image duplicate everything, and can fail each other

verifier/src/verification.rs:385-417,623-641,1280-1377

Neither ensure_image_downloaded nor load_or_compute_measurements coalesces identical in-flight work.

  • Duplicated work. N CVMs booting the same new image simultaneously — a fleet restart, the realistic case — all miss metadata.json, all download the full tarball, all shell out to sha256sum -c over it, all extract it, then all miss the measurement cache and all recompute the same Machine::measure_with_logs(). O(N) CPU and bandwidth where O(1) is correct.
  • Blocking the executor. extract_image_archive, prune_unlisted_image_files, load_measurements_from_cache, store_measurements_in_cache and compute_measurements are all synchronous fs_err + CPU work called directly from async fn with no spawn_blocking. Extracting a multi-hundred-MB image and SHA-384-ing a kernel + initrd each pin a Rocket worker for seconds; [default] workers = 8 in kms.toml, so eight concurrent cold boots stall the whole listener.
  • A race, not just waste. download_image finishes with remove_dir_all(dst_dir) then rename(extracted_dir, dst_dir). Two concurrent downloads of the same hash: A renames into place, B then removes A's directory — while a third request is mid-read_to_string(metadata_path) and fails with "Failed to read image metadata". Concurrent boots can fail each other.

This is explicitly not a decision cache: the image is addressed by its own hash and the measurement is a pure function of vm_config_cache_key. Neither is an authorization decision.

The spawn_blocking half stands alone and is small. The coalescing half (a Mutex<HashMap<String, Weak<Shared<…>>>> keyed by hash so the second caller awaits the first) is a judgement call — the number justifying it is "N concurrent cold boots cost N× and can fail each other". A bounded in-memory LRU in front of the file measurement cache would also remove a file read + JSON parse from every warm request.

Smaller

where what cost
main_service.rs:455-457 GetMeta re-reads and re-parses bootstrap-info.json from disk on every call — and it is immutable after bootstrap. With client_auth_mandatory() == false, any unauthenticated caller can drive it. (It also swallows a corrupt file through two chained .ok()s, which is a separate problem.) load once into KmsStateInner
main_service.rs:549-555, onboard_service.rs:176-186 Two whole-attestation clones to read one field. into_v1() takes self, so cloning is the path of least resistance — but the thing copied is quote + cert chain + event log, bounded by MAX_ATTESTATION_BYTES = 10 MiB. onboard_service.rs:176 clones and converts the whole attestation purely to match on platform's discriminant, then converts again. Typical cost is small; the bounded worst case is a 10 MiB memcpy per SignCert from an untrusted caller. a borrowing variant(&self) accessor
main_service.rs:127-134 ClearImageCache calls fs::remove_dir_all on an async handler; an image cache can be many GB. Admin-only and rare, but spawn_blocking is the right shape. S
IAppAuth.sol:36-46 advisoryIds (string[]) is ABI-encoded into every eth_call by both Ethereum backends and read by no contract. An unread string[] is the most expensive member of the struct to encode and send, on a call made 8 times per boot. (Whether to wire it into policy or drop it is a contract-deployment decision, tracked separately.) —
DstackKms.sol:107-109, DstackApp.sol:142-144 _emitPolicy does keccak256(bytes(policy)) where policy is always a string literal at the call site. Hoisting to bytes32 constant saves the copy and the hash. Being honest: ~100 gas on owner-only admin writes. Listed for completeness, not because it matters; it should only ride along with another contract change. negligible

Checked and found clean

So a later reader knows where not to look again: kms/src/crypto.rs (the cleanest file in the component — a verbatim pre-refactor reference implementation plus golden vectors in the test module, so the compatibility claim is machine-checked every run); admin_auth.rs and admin_service.rs; config.rs; send_request's bounded error context; onboard_service.rs's validate_onboarding_domain and ca_cert_expires_within; ra-rpc/src/ratls_client_verifier.rs (the expensive half of verification is deliberately kept out of the handshake and documented as such, so unauthenticated peers cannot drive quote verification from a TLS hello); cert-client/src/lib.rs; the gas shape of both contracts (no storage reads in loops, no unbounded iteration, calldata used correctly, __gap present in both — and the PolicyChanged audit-event design is a genuinely good addition, it makes the whole policy reconstructable from logs); the Manage.s.sol/Query.s.sol loops (one transaction per item is unavoidable given the contract's one-value-per-call setters); and the key derivation in get_app_key itself.

Lingua principale
Rust
Stelle
555
Fork
97
Merge medio
1g 3h
PR unite (30g)
195

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di Dstack-TEE/dstack

Tutte le issue di Dstack-TEE/dstack

Issue simili

Altre issue su Rust

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.