Skip to content

feat: log rejected payer funds and startup state - #21

Open
rickstaa wants to merge 4 commits into
mainfrom
rs/log-startup-and-payer-funds
Open

rickstaa wants to merge 4 commits into
mainfrom
rs/log-startup-and-payer-funds

Conversation

@rickstaa

Copy link
Copy Markdown
Member

An underfunded payer was rejected by the orchestrator with no server log, only a bare payer funds unavailable (400) on the client. A ticket face value above the claimable reserve (about 1/100 of the total reserve per recipient) then looked like a signer or wallet fault. The signer also printed nothing when healthy, so a successful start could not be told from a stuck one.

Orchestrator:

  • orchestrator started now carries onchain=true|false.
  • Rejecting a payer for low funds logs the payer, deposit, reserve and face value, in the existing event/operation/outcome shape.

Signer:

  • Logs a signer started line with the listen addresses.
  • Uses the same text log handler as the orchestrator, so both services share one format. This is the second commit and can be dropped separately.

No new test for the warning yet. TestStorageDefaultsAndChainCheck in signer also fails on a clean main.

rickstaa and others added 4 commits October 29, 2026 15:02
An underfunded payer was rejected with no server log, so a ticket face
value above the claimable reserve looked like a client or signer
fault. Log the payer, deposit, reserve and face value on rejection, and
report on-chain mode on the start line.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The signer printed nothing when healthy. Log listen addresses at
startup and use the same text handler as the orchestrator so both
services share one log format.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The warning used the default slog handler, so it printed in a different
format from the other orchestrator lines. Use the engine's store logger,
as the payment observed line does.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The logged reserve is the amount claimable by this orchestrator, not the
payer's total reserve. Say so, to avoid mistaking a funded wallet for a
logging error.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@rickstaa
rickstaa requested a review from j0sh October 29, 2026 14:16
@rickstaa
rickstaa marked this pull request as ready for review October 31, 2026 05:52
Copilot AI balanced review requested due to automatic review settings October 31, 2026 05:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Zero deposit or claimable reserve balances are rejected before the underfunding warning, leaving balance details unlogged; the warning must precede validation with nil guards while preserving rejection behavior.

1 open issue
What changed in this PR

This PR improves payment diagnostics and startup visibility for the orchestrator and signer.

Changes:

  • Logs payer balances and ticket face value when rejecting insufficient funds.
  • Adds on-chain status to the orchestrator startup log.
  • Adds a signer startup log and aligns its text logging format with the orchestrator.
File Description
signer/​app.go Configures text logging and reports startup listen addresses.
orchestrator/​payment_engine.go Adds a warning with funding details for payment rejection.
orchestrator/​app.go Includes on-chain status in the startup log.

Review effort: Balanced

Comment on lines +256 to +257
if e.store != nil && e.store.logger != nil {
e.store.logger.WarnContext(ctx, "orchestrator payer deposit or claimable reserve below ticket face value", "event", "payment", "operation", "receive", "outcome", "rejected", "payer", payer, "deposit", funds.Deposit, "claimable_reserve", funds.Reserve, "face_value", params.FaceValue)

This branch has not been deployed

No deployments
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.

3 participants