feat(reputation-registry): gate get_score behind pause circuit breaker - #185
Open
fridaypetra55-afk wants to merge 1 commit into
Open
Conversation
record_activity and other mutating entry points already checked pause::when_not_paused, but get_score did not, so a paused contract would still serve (potentially bogus) scores while scoring bugs were under investigation. get_score now returns Result<MoiScore, ReputationError> and rejects with ContractPaused when the admin has paused the contract, matching every other reputation-registry entry point. Also adds the missing testutils dev-dependency to reputation-registry/Cargo.toml (present in circle's Cargo.toml but absent here), which is required to build this package's own test suite in isolation. Closes cocor-tech#118
|
@fridaypetra55-afk 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! 🚀 |
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.
Summary
record_activityand the other mutating entry points already checkedpause::when_not_paused, butget_scoredid not — so a paused contract (e.g. after a scoring bug is discovered) would still happily serve scores while it's under investigation.get_scorenow returnsResult<MoiScore, ReputationError>and rejects withContractPausedwhen paused, consistent with every other entry point in the contract. The generated Soroban client keeps the ergonomicclient.get_score(...)panicking-on-error call site working unchanged;client.try_get_score(...)is used where the paused error needs to be observed.test_pause_blocks_get_scoretest mirroring the existingtest_pause_blocks_recordpattern.Note on pre-existing breakage (unrelated to this change)
While validating this fix I found
reputation-registry's own test suite does not currently compile onmaster, for reasons unrelated to this issue:lib.rsnever re-exportstypes::*(e.g.crate::MoiScore), whichtest.rsimports directly.test.rscallsclient.record(...)/client.try_record(...), but the contract only exposesrecord_activity.test.rsusesEvents::last()as an iterator, which isn't supported by the pinnedsoroban-sdkversion's testutils API.reputation-registry/Cargo.tomlwas also missing thetestutilsdev-dependency thatcircle/Cargo.tomlhas, so the package couldn't even attempt to build its tests — I've added that here since it's needed for this package to build its own test suite at all.This is likely why it was never caught: the CI workflow at
.github/workflows/*.ymltriggers onbranches: ["main"], but the repo's default branch ismaster, socargo test --workspace/ clippy have not actually been running in CI. I did not attempt to fix the pre-existingrecord/export/Events issues here since they're out of scope for #118 — happy to file that as a separate issue if useful. I did verifycargo build -p reputation-registry(the actual contract lib, non-test) compiles cleanly with this change.Closes #118