Jesse/orbis reporting#121
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## jesse/orbis-extras #121 +/- ##
======================================================
+ Coverage 49.77% 50.58% +0.81%
======================================================
Files 313 316 +3
Lines 17964 18405 +441
======================================================
+ Hits 8941 9310 +369
- Misses 8055 8087 +32
- Partials 968 1008 +40 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
x/orbis/keeper/msg_server.go (1)
50-67: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd a fallback for pre-existing rings
x/orbis/keeper/msg_server.go:50-67only snapshotsDefaultDemeritConfigonto new rings. Rings already in state will still decode with zeroDemeritConfig, andSubmitReport/NodeDemeritsnow read those fields directly. Add a migration or read-time default for legacy entries.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@x/orbis/keeper/msg_server.go` around lines 50 - 67, Legacy rings in state can still have an unset or zero-valued DemeritConfig, while SubmitReport and NodeDemerits now read it directly. Add a backward-compatible fallback in the keeper path: either migrate existing Ring entries to populate DemeritConfig from DefaultDemeritConfig, or make the read path in types.Ring-related logic default missing values before use. Use msg_server.go, SubmitReport, and NodeDemerits as the key places to update.
🧹 Nitpick comments (1)
x/orbis/keeper/params.go (1)
12-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for the new params contract.
GetParams()now manufactures defaults andSetParams()now rejects invalid configs, but these branches are still uncovered in the PR. A small keeper test for a missing params key plus an invalidDefaultDemeritConfigwould lock down theCreateRingfallback path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@x/orbis/keeper/params.go` around lines 12 - 28, Add keeper regression tests around GetParams and SetParams in the Keeper/params.go flow: verify GetParams returns types.DefaultParams when the params key is missing, and verify SetParams rejects an invalid types.Params containing a bad DefaultDemeritConfig. Use the existing GetParams, SetParams, and CreateRing fallback path to cover the new params contract and prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@proto/sourcehub/orbis/ring.proto`:
- Line 40: Backfill or default `demerit_config` for legacy `Ring` records so
`validateRing` does not fail on zero-valued protobuf decodes. Update the `Ring`
read/migration path around `validateRing` and the `demerit_config` field in
`ring.proto` to ensure existing stored rows get a non-zero config before
revalidation, either via a data migration or a read-time fallback.
In `@x/orbis/keeper/msg_server_test.go`:
- Around line 176-221: Add a regression test around MsgServer/CreateRing that
exercises the new params validation/defaulting path in x/orbis/keeper/params.go,
since TestMsgServer_CreateRingSnapshotsDemeritConfig only covers the happy path.
Extend TestMsgServer_CreateRingSnapshotsDemeritConfig (or add a nearby test) to
assert the ring inherits validated default demerit config from params when
omitted, and that invalid/normalized params are handled through the
params-related helpers used by CreateRing/GetRing. Reference the existing
CreateRing, SetParams, and GetRing flow so the new coverage clearly hits the
params logic and raises patch coverage for the params code.
In `@x/orbis/keeper/report.go`:
- Around line 356-393: `reportRingStateCanonicalBytes` is missing
`ring.DemeritConfig` from the canonical ring-state digest, so `SubmitReport` can
later apply a different penalty than was signed. Update the digest serialization
in `reportRingStateCanonicalBytes` to include the demerit configuration in a
deterministic way alongside the other ring fields, and make sure any related
canonical writer usage stays consistent so report verification binds to the
exact config used by `DemeritAmountForReportType`.
- Around line 127-133: The current verification in report.go only checks the
threshold ring signature and does not cryptographically bind the
reporter/exclusion constraints. Update the report verification flow around
validateReportCommitteeAuthorization and verifyThresholdSignature so the signed
payload includes the reporter and accused node keys, plus the signer set/bitmap
or equivalent participation proof, and have verifyThresholdSignature validate
that the required reporter signed and the accused was excluded. Ensure the
authorization policy and the signature verification use the same unique report
fields rather than relying on policy-only checks.
- Around line 117-125: The protocol-version validation in report handling is
using the current time instead of the signed report timestamp, which can
incorrectly reject valid reports around activation boundaries. Update the check
in report.go’s report validation path to derive the effective version from the
report’s signed timestamp rather than now, and keep the comparison against
payload.originProtocolVersion unchanged; use the existing
effectiveReportProtocolVersion helper with the report timestamp source used by
the report object.
- Around line 146-151: The report validation in report.go still allows
future-dated submissions because it only checks report.ObservedAt >
report.ExpiresAt and now > report.ExpiresAt. Update the validation in the report
checks to also reject cases where now is before report.ObservedAt, using the
existing report validity logic in the same switch so punitive reports cannot be
submitted before the outage time.
In `@x/orbis/keeper/validation_test.go`:
- Around line 13-18: The ring validation tests currently only cover the
happy-path fixture and do not exercise the new demerit checks in validateRing.
Update the table-driven cases in validation_test.go that build the ring fixture
with DemeritConfig and add invalid DemeritConfig variants there so validateRing
is forced through its new validation branches. Reuse the existing ring setup
around validateRing, types.DefaultDemeritConfig, and the ring fixture helpers so
the new cases stay close to the current PSS assertions while increasing
coverage.
---
Outside diff comments:
In `@x/orbis/keeper/msg_server.go`:
- Around line 50-67: Legacy rings in state can still have an unset or
zero-valued DemeritConfig, while SubmitReport and NodeDemerits now read it
directly. Add a backward-compatible fallback in the keeper path: either migrate
existing Ring entries to populate DemeritConfig from DefaultDemeritConfig, or
make the read path in types.Ring-related logic default missing values before
use. Use msg_server.go, SubmitReport, and NodeDemerits as the key places to
update.
---
Nitpick comments:
In `@x/orbis/keeper/params.go`:
- Around line 12-28: Add keeper regression tests around GetParams and SetParams
in the Keeper/params.go flow: verify GetParams returns types.DefaultParams when
the params key is missing, and verify SetParams rejects an invalid types.Params
containing a bad DefaultDemeritConfig. Use the existing GetParams, SetParams,
and CreateRing fallback path to cover the new params contract and prevent
regressions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4e10a1dc-bd75-4069-8c60-55f195f3f825
⛔ Files ignored due to path filters (10)
api/sourcehub/orbis/query_grpc.pb.gois excluded by!**/*.pb.goapi/sourcehub/orbis/ring.pb.gois excluded by!**/*.pb.goapi/sourcehub/orbis/tx.pb.gois excluded by!**/*.pb.goapi/sourcehub/orbis/tx_grpc.pb.gois excluded by!**/*.pb.gox/orbis/types/events.pb.gois excluded by!**/*.pb.gox/orbis/types/params.pb.gois excluded by!**/*.pb.gox/orbis/types/query.pb.gois excluded by!**/*.pb.gox/orbis/types/query.pb.gw.gois excluded by!**/*.pb.gw.gox/orbis/types/ring.pb.gois excluded by!**/*.pb.gox/orbis/types/tx.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (29)
api/sourcehub/orbis/events.pulsar.goapi/sourcehub/orbis/params.pulsar.goapi/sourcehub/orbis/query.pulsar.goproto/sourcehub/orbis/events.protoproto/sourcehub/orbis/params.protoproto/sourcehub/orbis/query.protoproto/sourcehub/orbis/ring.protoproto/sourcehub/orbis/tx.protox/orbis/keeper/demerits.gox/orbis/keeper/grpc_query.gox/orbis/keeper/msg_server.gox/orbis/keeper/msg_server_baseapp_test.gox/orbis/keeper/msg_server_test.gox/orbis/keeper/params.gox/orbis/keeper/report.gox/orbis/keeper/report_test.gox/orbis/keeper/reshare_sign_bytes_test.gox/orbis/keeper/store.gox/orbis/keeper/store_test.gox/orbis/keeper/validation.gox/orbis/keeper/validation_test.gox/orbis/module/autocli.gox/orbis/types/codec.gox/orbis/types/errors.gox/orbis/types/keys.gox/orbis/types/message_create_ring.gox/orbis/types/message_mutations_test.gox/orbis/types/message_submit_report.gox/orbis/types/params.go
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: test
🧰 Additional context used
🪛 ast-grep (0.44.0)
x/orbis/keeper/store.go
[warning] 115-115: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: uint32(len(ringID))
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
api/sourcehub/orbis/events.pulsar.go
[warning] 5470-5470: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: int32(wire >> 3)
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
x/orbis/keeper/report.go
[warning] 421-421: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: uint32(len(value))
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
[warning] 434-434: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: uint32(len(values))
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
api/sourcehub/orbis/query.pulsar.go
[warning] 7629-7629: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: int32(wire >> 3)
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
[warning] 8078-8078: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: int32(wire >> 3)
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
🪛 Buf (1.71.0)
proto/sourcehub/orbis/ring.proto
[error] 18-18: cannot find gogoproto.equal in this scope
(COMPILE)
[error] 40-40: cannot find gogoproto.nullable in this scope
(COMPILE)
proto/sourcehub/orbis/params.proto
[error] 6-6: imported file does not exist
(COMPILE)
[error] 12-12: cannot find amino.name in this scope
(COMPILE)
[error] 13-13: cannot find gogoproto.equal in this scope
(COMPILE)
[error] 15-15: cannot find DemeritConfig in this scope
(COMPILE)
[error] 15-15: cannot find gogoproto.nullable in this scope
(COMPILE)
proto/sourcehub/orbis/query.proto
[error] 61-61: cannot find google.api.http in this scope
(COMPILE)
proto/sourcehub/orbis/tx.proto
[error] 10-10: imported file does not exist
(COMPILE)
[error] 65-65: cannot find DemeritConfig in this scope
(COMPILE)
[error] 149-149: cannot find cosmos.msg.v1.signer in this scope
(COMPILE)
[error] 151-151: cannot find gogoproto.nullable in this scope
(COMPILE)
🔇 Additional comments (13)
x/orbis/keeper/store_test.go (1)
42-101: LGTM!x/orbis/keeper/report_test.go (1)
1-692: LGTM!x/orbis/keeper/msg_server_test.go (1)
84-84: LGTM!x/orbis/keeper/msg_server_baseapp_test.go (1)
100-104: LGTM!Also applies to: 211-216
x/orbis/keeper/reshare_sign_bytes_test.go (1)
43-44: LGTM!Also applies to: 78-79
proto/sourcehub/orbis/ring.proto (1)
16-23: LGTM!proto/sourcehub/orbis/params.proto (1)
6-15: LGTM!proto/sourcehub/orbis/tx.proto (1)
10-10: LGTM!Also applies to: 28-28, 65-65, 148-175
proto/sourcehub/orbis/events.proto (1)
71-78: LGTM!x/orbis/types/message_submit_report.go (1)
9-9: LGTM!Also applies to: 11-49
x/orbis/types/message_create_ring.go (1)
38-42: LGTM!x/orbis/types/codec.go (1)
20-20: LGTM!api/sourcehub/orbis/params.pulsar.go (1)
148-152: 🩺 Stability & AvailabilityNo change needed for
default_demerit_configGet.DemeritConfig.ProtoReflect()handles nil receivers, sovalue.ProtoReflect()on an unset field returns an empty reflection view instead of panicking.> Likely an incorrect or invalid review comment.
| func TestMsgServer_CreateRingSnapshotsDemeritConfig(t *testing.T) { | ||
| k, authKeeper, ctx := setupOrbisKeeper(t) | ||
| ctx = ctx.WithValue(appparams.ExtractedDIDContextKey, testDID) | ||
|
|
||
| creatorAddr, _ := testAccountWithPubKey(t, ctx, authKeeper) | ||
| _, peer1Key := setupPeerWithNodeInfo(t, k, authKeeper, ctx, "12D3KooWPeer1") | ||
| policyID := createOrbisRingPolicy(t, k, ctx, creatorAddr) | ||
|
|
||
| defaultConfig := types.DemeritConfig{ | ||
| NodeOfflineDemerits: 4, | ||
| ResetIntervalSeconds: 12, | ||
| } | ||
| require.NoError(t, k.SetParams(ctx, types.NewParams(defaultConfig))) | ||
|
|
||
| defaultResp, err := k.CreateRing(ctx, &types.MsgCreateRing{ | ||
| Creator: creatorAddr, | ||
| PeerNodeKeys: []string{peer1Key}, | ||
| Threshold: 1, | ||
| PssInterval: types.MinPSSIntervalSeconds, | ||
| PolicyId: policyID, | ||
| }) | ||
| require.NoError(t, err) | ||
| require.Equal(t, defaultConfig, k.GetRing(ctx, defaultResp.RingId).DemeritConfig) | ||
|
|
||
| require.NoError(t, k.SetParams(ctx, types.NewParams(types.DemeritConfig{ | ||
| NodeOfflineDemerits: 8, | ||
| ResetIntervalSeconds: 24, | ||
| }))) | ||
| require.Equal(t, defaultConfig, k.GetRing(ctx, defaultResp.RingId).DemeritConfig) | ||
|
|
||
| explicitConfig := types.DemeritConfig{ | ||
| NodeOfflineDemerits: 7, | ||
| ResetIntervalSeconds: 60, | ||
| } | ||
| explicitResp, err := k.CreateRing(ctx, &types.MsgCreateRing{ | ||
| Creator: creatorAddr, | ||
| PeerNodeKeys: []string{peer1Key}, | ||
| Threshold: 1, | ||
| PssInterval: types.MinPSSIntervalSeconds, | ||
| PolicyId: policyID, | ||
| XNonce: &types.MsgCreateRing_Nonce{Nonce: "explicit-demerit-config"}, | ||
| DemeritConfig: &explicitConfig, | ||
| }) | ||
| require.NoError(t, err) | ||
| require.Equal(t, explicitConfig, k.GetRing(ctx, explicitResp.RingId).DemeritConfig) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add one params validation/defaulting regression here too.
This only proves the happy-path snapshot behavior. The new x/orbis/keeper/params.go paths are still unexercised, and the PR metadata still shows that file at 0% patch coverage.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@x/orbis/keeper/msg_server_test.go` around lines 176 - 221, Add a regression
test around MsgServer/CreateRing that exercises the new params
validation/defaulting path in x/orbis/keeper/params.go, since
TestMsgServer_CreateRingSnapshotsDemeritConfig only covers the happy path.
Extend TestMsgServer_CreateRingSnapshotsDemeritConfig (or add a nearby test) to
assert the ring inherits validated default demerit config from params when
omitted, and that invalid/normalized params are handled through the
params-related helpers used by CreateRing/GetRing. Reference the existing
CreateRing, SetParams, and GetRing flow so the new coverage clearly hits the
params logic and raises patch coverage for the params code.
| effectiveVersion := effectiveReportProtocolVersion(ring, now) | ||
| if payload.originProtocolVersion != effectiveVersion { | ||
| return nil, errorsmod.Wrapf( | ||
| types.ErrInvalidReport, | ||
| "report origin protocol version %d is not effective for ring %s", | ||
| payload.originProtocolVersion, | ||
| report.RingId, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Key protocol-version checks off the report timestamp, not submit time.
Using now here rejects reports that were observed before an activation boundary but submitted within the 120-second TTL after it. originProtocolVersion should be evaluated against the signed report timestamp.
Suggested fix
- effectiveVersion := effectiveReportProtocolVersion(ring, now)
+ effectiveVersion := effectiveReportProtocolVersion(ring, report.ObservedAt)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| effectiveVersion := effectiveReportProtocolVersion(ring, now) | |
| if payload.originProtocolVersion != effectiveVersion { | |
| return nil, errorsmod.Wrapf( | |
| types.ErrInvalidReport, | |
| "report origin protocol version %d is not effective for ring %s", | |
| payload.originProtocolVersion, | |
| report.RingId, | |
| ) | |
| } | |
| effectiveVersion := effectiveReportProtocolVersion(ring, report.ObservedAt) | |
| if payload.originProtocolVersion != effectiveVersion { | |
| return nil, errorsmod.Wrapf( | |
| types.ErrInvalidReport, | |
| "report origin protocol version %d is not effective for ring %s", | |
| payload.originProtocolVersion, | |
| report.RingId, | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@x/orbis/keeper/report.go` around lines 117 - 125, The protocol-version
validation in report handling is using the current time instead of the signed
report timestamp, which can incorrectly reject valid reports around activation
boundaries. Update the check in report.go’s report validation path to derive the
effective version from the report’s signed timestamp rather than now, and keep
the comparison against payload.originProtocolVersion unchanged; use the existing
effectiveReportProtocolVersion helper with the report timestamp source used by
the report object.
| case report.ObservedAt > report.ExpiresAt: | ||
| return errorsmod.Wrap(types.ErrInvalidReport, "report observed_at is after expires_at") | ||
| case report.ExpiresAt-report.ObservedAt != ReportTTLSeconds: | ||
| return errorsmod.Wrap(types.ErrInvalidReport, "invalid report validity window") | ||
| case now > report.ExpiresAt: | ||
| return errorsmod.Wrap(types.ErrInvalidReport, "report has expired") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject future-dated reports.
now < report.ObservedAt still passes here, so a committee can submit a punitive report before the alleged outage time arrives.
Suggested fix
case report.ObservedAt > report.ExpiresAt:
return errorsmod.Wrap(types.ErrInvalidReport, "report observed_at is after expires_at")
+ case report.ObservedAt > now:
+ return errorsmod.Wrap(types.ErrInvalidReport, "report observed_at is in the future")
case report.ExpiresAt-report.ObservedAt != ReportTTLSeconds:
return errorsmod.Wrap(types.ErrInvalidReport, "invalid report validity window")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case report.ObservedAt > report.ExpiresAt: | |
| return errorsmod.Wrap(types.ErrInvalidReport, "report observed_at is after expires_at") | |
| case report.ExpiresAt-report.ObservedAt != ReportTTLSeconds: | |
| return errorsmod.Wrap(types.ErrInvalidReport, "invalid report validity window") | |
| case now > report.ExpiresAt: | |
| return errorsmod.Wrap(types.ErrInvalidReport, "report has expired") | |
| case report.ObservedAt > report.ExpiresAt: | |
| return errorsmod.Wrap(types.ErrInvalidReport, "report observed_at is after expires_at") | |
| case report.ObservedAt > now: | |
| return errorsmod.Wrap(types.ErrInvalidReport, "report observed_at is in the future") | |
| case report.ExpiresAt-report.ObservedAt != ReportTTLSeconds: | |
| return errorsmod.Wrap(types.ErrInvalidReport, "invalid report validity window") | |
| case now > report.ExpiresAt: | |
| return errorsmod.Wrap(types.ErrInvalidReport, "report has expired") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@x/orbis/keeper/report.go` around lines 146 - 151, The report validation in
report.go still allows future-dated submissions because it only checks
report.ObservedAt > report.ExpiresAt and now > report.ExpiresAt. Update the
validation in the report checks to also reject cases where now is before
report.ObservedAt, using the existing report validity logic in the same switch
so punitive reports cannot be submitted before the outage time.
| Id: "ring-1", | ||
| PeerNodeKeys: []string{"020000000000000000000000000000000000000000000000000000000000000000"}, | ||
| Threshold: 1, | ||
| PssInterval: types.MinPSSIntervalSeconds, | ||
| PolicyId: "policy-1", | ||
| DemeritConfig: types.DefaultDemeritConfig(), |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Cover invalid DemeritConfig cases here as well.
These fixture updates preserve the old PSS assertions, but they still never hit the new demerit validation in validateRing. The PR metadata still shows x/orbis/keeper/validation.go at 0% patch coverage.
Also applies to: 32-37
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@x/orbis/keeper/validation_test.go` around lines 13 - 18, The ring validation
tests currently only cover the happy-path fixture and do not exercise the new
demerit checks in validateRing. Update the table-driven cases in
validation_test.go that build the ring fixture with DemeritConfig and add
invalid DemeritConfig variants there so validateRing is forced through its new
validation branches. Reuse the existing ring setup around validateRing,
types.DefaultDemeritConfig, and the ring fixture helpers so the new cases stay
close to the current PSS assertions while increasing coverage.
* orbis module extras * remove uneeded check from finalize * remove redundant check * more redundant check * refine checks * fix state deletion in finalize ring * delete ring * Jesse/orbis reporting (#121) * Add reporting * clean * add settable demerits * add querys * window demerit reset * proto * more tests * node demerits rpc change * coderabbit * coderabbit * demerit genesis * add session id to reporting * more tests * expire old reports and sessions from storage * coderabbit * add orbis to end block
2 participants
No description provided.