Skip to content

fix(cli): admin payout-issue reports success even when payout_bounty loses the amount#846

Open
ebios-star wants to merge 1 commit intoentrius:testfrom
ebios-star:fix/admin-payout-success-with-zero-amount
Open

fix(cli): admin payout-issue reports success even when payout_bounty loses the amount#846
ebios-star wants to merge 1 commit intoentrius:testfrom
ebios-star:fix/admin-payout-success-with-zero-amount

Conversation

@ebios-star
Copy link
Copy Markdown
Contributor

Summary

Closes #844.

`IssueCompetitionContractClient.payout_bounty` returns three different values:

Return Meaning
positive int success with known amount
`0` success but the pre-payout `get_issue` couldn't reach the chain (RPC flake / dropped websocket), so the function lost track of the bounty even though the extrinsic succeeded
`None` extrinsic failed

`gitt admin payout-issue` collapsed the last two cases with `if result:` and printed "Payout failed." — exiting non-zero — when the on-chain state had actually paid out. Operators retried the call and hit the contract's already-Completed revert, reinforcing the wrong belief.

The CLI already reads the issue once before submitting (line 139), so when `payout_bounty` returns `0` we can fall back to that locally-known bounty for the success message instead of mis-labelling the round as a failure.

After this change:

  • `if result is not None` gates success/failure — `0` is correctly recognized as success.
  • The success message prefers the contract client's amount when it's positive, and falls back to `issue.bounty_amount` when it's `0`.
  • The contract_client API stays unchanged.

Related Issues

Closes #844.

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Other

Testing

  • New `TestAdminPayoutSuccessWithUnknownAmount` covers all three cases:
    • `payout_bounty` returns 0 → success message uses local bounty (`42.0000 ALPHA`)
    • `payout_bounty` returns None → "Payout failed." + exit 1 (existing contract preserved)
    • `payout_bounty` returns positive → uses that amount (forward-compatible)
  • `uv run --extra dev pytest tests/` — 646 tests pass (643 baseline + 3 new).

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Changes are documented (if applicable)

…loses the amount

`IssueCompetitionContractClient.payout_bounty` returns three values:

- a positive int → success with known amount
- 0             → success but the pre-payout `get_issue` couldn't reach the
                  chain (RPC flake / dropped websocket), so the function lost
                  track of the bounty even though the extrinsic succeeded
- None          → extrinsic failed

`gitt admin payout-issue` collapsed cases 2 and 3 with `if result:` and
printed "Payout failed." — exiting non-zero — when the on-chain
state had actually paid out. Operators retried the call and hit the
contract's already-Completed revert, reinforcing the wrong belief.

The CLI already reads the issue once before submitting (line 139), so
when `payout_bounty` returns 0 we can fall back to that locally-known
bounty for the success message instead of mis-labelling the round as
a failure.

Use `result is not None` to gate success/failure, prefer the contract
client's amount when it's positive, and fall back to `issue.bounty_amount`
when it's 0. The contract_client API stays unchanged.

Add `TestAdminPayoutSuccessWithUnknownAmount` covering all three cases:
the 0-return success path, the None-return failure path, and the
positive-return path (forward-compatible with any future re-read).

Closes entrius#844.
@xiao-xiao-mao xiao-xiao-mao Bot added the bug Something isn't working label Apr 28, 2026
@ebios-star
Copy link
Copy Markdown
Contributor Author

@anderdc @LandynDev — friendly ping for review when you have a moment.

Closes #844. gitt admin payout-issue was reporting "Payout failed." + exit 1 when the on-chain transaction had actually succeeded — because payout_bounty returns 0 (success but unknown amount) when its internal get_issue flakes mid-call, and the CLI's if result: truthiness check collapsed that with None (genuine failure). Operators retried the call and hit the contract's already-Completed revert.

Switched to if result is not None, falling back to the bounty we already read pre-payout when the contract client lost track. contract_client API stays unchanged. 3 new regression tests covering all three return values.

Happy to rebase if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

1 participant