Repository navigation
test: pin the claim term of the inline receipt check - #562
Merged
Merged
Conversation
records_inline_publication accepts a stored receipt only when its
generation follows the request's claim. No test failed with that term
removed, because the reused-identity tests change the body or the
revision, which the other terms already refuse.
The new test publishes create-only, then reuses the operation id,
revision and body with ReplaceOnly { expected_generation: 1 }. The
stored receipt matches every other input, and its generation 1 does not
follow a replace of generation 1, so only the claim term refuses it. The
client keeps the server's RequestReplayMismatch, aborts nothing, and the
path keeps generation 1 and the first body. With the term removed, the
client returns the create-only receipt as a replayed success instead.
Signed-off-by: wchwawa <wch19961116@gmail.com>
wchwawa
changed the title
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.
TLDR
A unit test now pins the generation term of the inline receipt check that #555
added, so dropping that term fails a test instead of passing silently. No
product code changes.
Problem
Relates to #551 and #555.
When an inline publication's request fails,
records_inline_publicationaccepts the stored receipt as a replay only if it records this publication,
including a generation that follows the request's claim:
CreateOnlygives1, and
ReplaceOnlyorAppend { Some(..) }gives the expected generationplus one. No test failed with the generation term removed. The #555 tests
reuse an id only with a different body or revision, which other terms already
refuse.
Consequences
A refactor could drop the term without any test noticing. The client would
then report a create-only receipt as the success of a replace that reused the
same id, revision, and body.
Example
Publish create-only, then reuse the same operation id, revision, and body with
ReplaceOnly { expected_generation: 1 }. The client must fail with an inlineRequestReplayMismatchand no abort, and the path must still hold generation1 and the first body. With the term removed, the client instead returns
generation: 1, replayed: trueas a success.Solution
One test in the
nokv-clientartifact tests,an_inline_identity_reused_with_another_claim_is_refused_without_a_stale_receipt.It reuses
ScriptedArtifactTransport, whose begin step refuses a storedrequest with a different condition, as the engine does.
Gain
The claim term of the replay check is guarded by a test that fails without it.
Tradeoffs
None beyond one more unit test.
Validation
With Rust 1.99.1:
cargo fmt --all -- --check: clean.cargo clippy -p nokv-client --all-targets -- -D warnings: passes.cargo test -p nokv-client: 129 passed.&& generation_follows_claimfromrecords_inline_publication:mainwith the mutant passes 128; this branchwith the mutant passes 128 and fails 1, the new test; this branch passes 129.
rust.ymlruns, and theWorkbench contract tests, since nothing Workbench-facing changes.