Skip to content

Conversation

@David-code-tang
Copy link
Contributor

Implements Wave2 bounty #130: adds dependency-light preflight validation for /wallet/transfer and /wallet/transfer/signed, hardens /wallet/transfer to avoid 500s on malformed/non-finite inputs, includes CLI checker and docs.\n\nTests:\n- PYTHONPATH=. python3 -m unittest node/tests/test_payout_preflight.py -v\n\nDocs:\n- docs/PAYOUT_PREFLIGHT.md\n

@Scottcjn
Copy link
Owner

Strong patch set and the preflight tests pass locally. One blocker before merge:

Deployment compatibility: this PR imports from node.payout_preflight .... Our production nodes run the main file directly (script-style), where sys.path[0] points at the script directory. In that mode, node.* imports fail (ModuleNotFoundError: No module named 'node').

Please switch to a deployment-safe import pattern, e.g. try local-module import first (or vice versa):

try:
    from payout_preflight import ...
except ImportError:
    from node.payout_preflight import ...

(or equivalent robust fallback).

After that I can fast-track merge.

@Scottcjn
Copy link
Owner

Status: this is functionally superseded by PR #127 (same preflight layer + stronger 2-phase commit fix).

Also still has deployment-compat import issue (from node.payout_preflight ...) for our single-file runtime layout.

Recommendation:

  1. either close this PR in favor of Security: signed transfers must use pending_ledger 2-phase commit #127, or
  2. refactor imports to single-file compatible pattern and scope this PR to preflight-only so it can be reviewed independently.

@Scottcjn
Copy link
Owner

Closing — this PR is superseded by #127 which addresses the same bounty (#59) with a more complete implementation. Please continue work in #127.

@Scottcjn
Copy link
Owner

Closing as superseded by merged PR #127 (includes the same preflight layer + additional 2-phase commit hardening for signed transfers).

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