Skip to content

fix: correct cs-bit polarity in compound-wait and chained request logic #63

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SoundMatt wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: correct cs-bit polarity in compound-wait and chained request logic #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions include/rcp/request.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,16 +355,16 @@ enum class CompoundWaitCheck : uint8_t {
};

constexpr CompoundWaitCheck compound_wait_check_of(bool cs) noexcept {
return cs ? CompoundWaitCheck::Immediate : CompoundWaitCheck::AfterChangeOnly;
return cs ? CompoundWaitCheck::AfterChangeOnly : CompoundWaitCheck::Immediate;
}

// should_execute_chained answers, for one chained successor: given its own
// `cs` bit and whether its predecessor finished in error, should this
// successor still run? cs=true means "execute regardless"; cs=false means
// successor still run? cs=false means "execute regardless"; cs=true means
// "abort if the predecessor errored". A predecessor that did *not* error
// always permits the successor to run, independent of cs.
constexpr bool should_execute_chained(bool cs, bool predecessor_errored) noexcept {
return cs || !predecessor_errored;
return !cs || !predecessor_errored;
}

// ── Optional-feature bundling ──────────────────────────────────────────────────
Expand Down
20 changes: 10 additions & 10 deletions tests/test_request.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ TEST_CASE("finalize with errored=true aborts a chained successor whose cs forbid
RequestRecord successor;
successor.transaction_num = 2;
successor.chained_predecessor = uint8_t{1};
successor.cs = false; // abort-on-predecessor-error
successor.cs = true; // abort-on-predecessor-error

REQUIRE_FALSE(ledger.submit(predecessor));
REQUIRE_FALSE(ledger.submit(successor));
Expand All @@ -484,7 +484,7 @@ TEST_CASE("finalize with errored=true does not abort a chained successor with cs
RequestRecord successor;
successor.transaction_num = 2;
successor.chained_predecessor = uint8_t{1};
successor.cs = true; // execute regardless
successor.cs = false; // execute regardless

REQUIRE_FALSE(ledger.submit(predecessor));
REQUIRE_FALSE(ledger.submit(successor));
Expand Down Expand Up @@ -518,20 +518,20 @@ TEST_CASE("finalize with errored=false never aborts a chained successor regardle
// ── The `cs` field's dual meaning ─────────────────────────────────────────────

TEST_CASE("compound_wait_check_of maps cs to Immediate vs AfterChangeOnly", "[request][REQ-SEQ-008]") {
REQUIRE(compound_wait_check_of(true) == CompoundWaitCheck::Immediate);
REQUIRE(compound_wait_check_of(false) == CompoundWaitCheck::AfterChangeOnly);
REQUIRE(compound_wait_check_of(false) == CompoundWaitCheck::Immediate);
REQUIRE(compound_wait_check_of(true) == CompoundWaitCheck::AfterChangeOnly);
}

TEST_CASE("should_execute_chained: cs=true always executes regardless of predecessor outcome",
TEST_CASE("should_execute_chained: cs=false always executes regardless of predecessor outcome",
"[request][REQ-SEQ-008]") {
REQUIRE(should_execute_chained(/*cs=*/true, /*predecessor_errored=*/true));
REQUIRE(should_execute_chained(/*cs=*/true, /*predecessor_errored=*/false));
REQUIRE(should_execute_chained(/*cs=*/false, /*predecessor_errored=*/true));
REQUIRE(should_execute_chained(/*cs=*/false, /*predecessor_errored=*/false));
}

TEST_CASE("should_execute_chained: cs=false aborts only when the predecessor errored",
TEST_CASE("should_execute_chained: cs=true aborts only when the predecessor errored",
"[request][REQ-SEQ-008]") {
REQUIRE_FALSE(should_execute_chained(/*cs=*/false, /*predecessor_errored=*/true));
REQUIRE(should_execute_chained(/*cs=*/false, /*predecessor_errored=*/false));
REQUIRE_FALSE(should_execute_chained(/*cs=*/true, /*predecessor_errored=*/true));
REQUIRE(should_execute_chained(/*cs=*/true, /*predecessor_errored=*/false));
}

// ── RequestErrc category sanity ─────────────────────────────────────────────
Expand Down
Loading