Hacktoberfest 2026: những issue maintainer đã đánh dấu cho tháng Mười, đang mở và phù hợp người mới. Xem issue Hacktoberfest

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

Đang mở
#1,259 0 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
5/5
Thời gian dự kiến
Hơn một tuần
Mức phù hợp với người mới
32/100
Loại issue
Tái cấu trúc
Độ rõ ràng
Khá rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
rust, solidity, typescript
Lĩnh vực
api, backend, performance, security

Hướng nghiên cứu

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.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

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.

Ngôn ngữ chính
Rust
Star
551
Fork
97
Merge trung bình
1 ngày 8 giờ
Pull request đã merge (30 ngày)
182

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của Dstack-TEE/dstack

Tất cả issue của Dstack-TEE/dstack

Issue tương tự

Thêm issue về Rust

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.