Skip to content

fix: keep current values for omitted set-auto-renewed-config flags - #40

Open
owenwahlgren wants to merge 2 commits into
mainfrom
fix/set-auto-renewed-config-optional-auto-compound
Open

fix: keep current values for omitted set-auto-renewed-config flags#40
owenwahlgren wants to merge 2 commits into
mainfrom
fix/set-auto-renewed-config-optional-auto-compound

Conversation

@owenwahlgren

Copy link
Copy Markdown
Collaborator

set-auto-renewed-config required both --period and --auto-compound, so changing either one meant restating the other from memory. SetAutoRenewedValidatorConfigTx always rewrites both fields, so a wrong guess silently overwrites the setting you were not trying to change.

The exit case shows it most clearly: stopping renewal with --period 0 still forced you to supply an auto-compound percentage that is about to stop mattering.

What changed

  • Both flags are now optional, and at least one must be given. Passing neither is an error rather than a no-op transaction.
  • An omitted flag inherits its current on-chain value, read from the same getCurrentValidators call that already fetched the validator authority, so this costs no extra request.
  • The inherited value is printed before submission, and the confirmation output now reflects the effective values rather than the raw flag.
  • A validator already set to exit keeps a zero next period when only --auto-compound changes, so tweaking the percentage cannot accidentally restart renewal.

GetAutoRenewedValidatorAuthority is kept as a thin wrapper over the new GetAutoRenewedValidatorConfig so existing callers and tests are unaffected.

Verification

go build ./..., gofmt -l . clean, full go test ./... green. Three new tests cover parsing the current values, the already-exiting validator case, and an unparseable share count. docs/usage.md updated to match.

Comment thread pkg/pchain/pchain.go Fixed
Both --period and --auto-compound were required, so changing one meant restating
the other. Restating it from memory risks overwriting it, since
SetAutoRenewedValidatorConfigTx always rewrites both fields.

Both flags are now optional and at least one is required. Omitted flags inherit
the current on-chain value, read from the same getCurrentValidators call that
already fetched the validator authority. A validator already set to exit keeps a
zero next period when only --auto-compound changes.
CodeQL flagged the uint64 to time.Duration conversion (go/incorrect-integer-conversion,
high). An oversized nextPeriod wrapped to a negative duration, which then slipped
past the caller's minimum check because that only guards period > 0, so a bogus
value could have been submitted as the next cycle length.

Bound the value against the largest whole-second duration an int64 can hold and
error out above it.
@owenwahlgren
owenwahlgren force-pushed the fix/set-auto-renewed-config-optional-auto-compound branch from 7e7ec0c to 8a808db Compare July 28, 2026 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants