Conversation
There was a problem hiding this comment.
🟡 Changes recommended
nextSerial’s default-host-clock path currently bypasses the same millisecond validation applied to explicit inputs, which can silently coerce invalid host clock values instead of refusing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR expands the onomancy_wasm public surface so downstream consumers can reuse the project’s canonical TXT record selection / serial minting rules and identify exactly which build artifact is loaded.
Changes:
- Export RRset TXT record classification and “next serial” logic through Wasm, with wasm/host tests to pin the JS boundary and the TypeScript shapes.
- Add
buildInfo()(version + embedded git revision) and the build-script plumbing to embed a non-empty revision string. - Factor and document signer capability types in the TypeScript custom section, and align serial / generation-key encoding helpers for shared use.
File summaries
| File | Description |
|---|---|
| onomancy_wasm/tests/records.rs | New wasm-bindgen tests that pin the JS-visible object shapes and refusal reason codes for record classification + serial minting. |
| onomancy_wasm/tests/browser.rs | Adds a wasm test asserting buildInfo() reports the package version and a non-empty revision. |
| onomancy_wasm/src/shapes.rs | Extends the TypeScript-shape externs to include RecordClassification and BuildInfo. |
| onomancy_wasm/src/shapes.d.ts | Adds BuildInfo, Signing/SignBytes, and RecordClassification / RecordCandidate declarations to the published TS custom section. |
| onomancy_wasm/src/records.rs | New Wasm-exported RRset classification and nextSerial implementation, plus host-side property/unit tests. |
| onomancy_wasm/src/lib.rs | Exposes the new records module and adds buildInfo() plus VERSION/REVISION constants. |
| onomancy_wasm/src/issue.rs | Updates issuance docs to describe the signer capability contract and adds a drift test for the TS declarations. |
| onomancy_wasm/README.md | Documents new exports (buildInfo(), signer contract wording, etc.). |
| onomancy_wasm/package.json | Bumps the JS package version to 0.3.0. |
| onomancy_wasm/Cargo.toml | Bumps the crate version to 0.3.0. |
| onomancy_wasm/build.rs | New build script to embed a git-derived or env-provided ONOMANCY_GIT_REV into the build. |
| onomancy_protocol/src/verifier/state.rs | Makes SKEW_MS public so one-shot consumers can apply the same skew/deferral bound. |
| onomancy_dnssec/src/txt/serial.rs | Adds Serial::next (publisher rule) plus SerialExhausted, with property testing. |
| onomancy_dnssec/src/txt/record.rs | Switches TXT record formatting to use GenerationKey’s display (canonical base64). |
| onomancy_dnssec/src/txt/generation_key.rs | Implements Display for GenerationKey as canonical padded base64. |
| onomancer/src/record.rs | Updates CLI serial behavior to use the shared publisher rule (--after / Serial::next) and surfaces exhaustion as an error. |
| Cargo.lock | Updates the onomancy_wasm lock entry to 0.3.0. |
Review details
Suppressed comments (1)
onomancy_wasm/src/records.rs:287
host_now_ms()currently castsDate::now()directly tou64, which can silently coerce unexpected host clock values (e.g., negative pre-epoch timestamps) instead of enforcing the same “non-negative integer that fits u64” contract asmilliseconds(). Returning aResulthere keeps the behavior consistent and letsnext_serialrefuse clearly when the host clock is invalid.
/// The host clock in milliseconds.
#[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)] // host clock fits
fn host_now_ms() -> u64 {
Date::now() as u64
}
- Files reviewed: 15/17 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
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.
Downstream consumers could use these instead of reimplementing themselves