feat(sdk): gate sealed bids on encoding and tlock commitment before c… - #458
Merged
karagozemin merged 4 commits intoOct 1, 2026
Merged
karagozemin merged 4 commits into
karagozemin merged 4 commits into
Conversation
…ommit The SDK validated only size and encoding, so a blob that decoded but was committed to the wrong value would reach the chain and never open. The commitment check existed only in the post-reveal receipt verifier. tlock now owns the sealed-payload wire format (payload.ts): parseSealedPayload reads a tlock ciphertext's structure — armor, age v1 header, `-> tlock <round> <hash>` stanza, MAC line — without decrypting or touching the network, and reports the round, chain hash, and implied plaintext length. commitmentMatches in commitment.ts is the shared acceptance rule both sides call. validateSealedBid composes three layers: encoding, length (32-byte commitment, 48-byte be16(value)||nonce preimage), and — when the caller supplies the optional `binding` of value/nonce/round — the re-derived commitment. commit() runs it before any contract call. Errors never carry the bid value. Also fixes three pre-existing defects found on the way: - options.encoding was accepted and documented but never read; the test for it had been failing on main and was not in the package test script. - Empty auditor blobs were rejected, though sealBid emits one whenever no identity is disclosed and the contract takes auditor_blob as optional Bytes. - A wrong-width commitment reported both invalid_commitment_length and commitment_mismatch. Swaps are caught when the caller supplies the binding of the seal it holds. A self-consistent but wrong value/nonce pair still passes, since detecting that would require decrypting — that limit is documented by a named test. Tests use real sealBid output via a stub Drand client, so the gate is checked against the sealer it must agree with. sdk 110/110, tlock 46/46.
…cation Resolves the seven conflicting files and repairs the merge result. Conflict resolutions: - packages/tlock/src/payload.ts + payload.test.ts: add/add. Upstream created these for a versioned binary payload envelope; my age-armor ciphertext parser is a different concern, so upstream keeps the path and mine moves to ciphertext.ts. Export names do not collide, so both modules coexist. - packages/tlock/package.json, packages/sdk/package.json: both sides extended the test script. Took upstream's superset and added ciphertext.test.ts. - packages/tlock/src/commitment.test.ts, packages/sdk/src/encrypted-blob.test.ts: both sides appended tests. Kept both. - packages/sdk/src/encrypted-blob.ts: kept upstream's SPDX header and their `encoding` implementation, which is equivalent to mine and which the merged tests assert against. The target branch did not compile at its own HEAD, so the merge inherited breakage from commit 88ef67b that had to be repaired to get a green build: - client.ts: stray `*/` closing a JSDoc block early - client.ts: missing `}` after `#validateAssetConfig`'s return - client.ts: `decimals: 77 <garbage text>` plus a stray label; restored to 7, matching native XLM in ASSET_FIXTURES and the `?? 7` fallback beside it - errors.ts: SubRosaAssetValidationError left unterminated, leaving two classes unclosed - client.ts: `this.#config.assetConfig` referenced a field that does not exist; `#assetConfig` was already declared alongside it - client.ts: SubRosaAssetValidationError constructed but never imported - client.ts: create_round called twice, redeclaring `tx`; kept the call wrapped in #validatedContractCall and folded asset_config into it - client.ts: `types.RoundAssetConfig` was an unimported namespace; declared the shape locally, mirroring contracts/round/src/types.rs, since the generated bindings predate the asset_config argument. Flagged in a comment to drop the local type and the call-site cast once bindings are regenerated. - public-api-snapshot.test.ts: SubRosaAssetValidationError was exported but never listed, failing on the target branch before this merge. sdk 234/234, tlock 90/90, bindings 17/17, keeper 73/73, agent 38/38, appraisal-api 49/49, auction-template 5/5; all typechecks clean.
|
@Damilorlar Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
`create_round` requires a trailing `RoundAssetConfig`, but the 88ef67b migration left every test call site on the old 7-argument signature, so `cargo test -p sub-rosa-round` failed to compile with E0061 (wrong argument count) and E0425 (RoundAssetConfig and soroban_sdk::String never imported). Adds a `native_xlm` test helper returning a native XLM config — the default for tests that don't exercise SAC binding — and threads it through all nine call sites: five in test.rs, four in error_paths.rs. `create_round` stores the config without validating its fields, so a well-formed native config is enough for every existing test. Imports `RoundAssetConfig` from crate::types and `String` from soroban_sdk, and re-exports the helper for error_paths.rs to use. Verified by static analysis: every call site now passes 8 arguments, brace balance is clean, and Round is constructed only in lib.rs where the field is already populated. NOT verified by cargo test — no Rust toolchain is installed in this environment, so this needs a CI run to confirm.
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
2 participants
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.
##closed #396