Repository navigation
Conversation
…drawals Escrow.withdrawals in the Python SDK validated `include` before resolving the wallet address, the reverse of the TypeScript SDK. Calling withdrawals(include="") with no wallet address therefore raised ValidationError in Python but MissingWalletAddress in TypeScript. Resolve the wallet address first, matching the TypeScript SDK (and Python's own approve_tx), and add regression tests for the validation order. Closes pmxt-dev#2353
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by winshyon
Description
Escrow.withdrawalsvalidated its inputs in opposite orders in the two SDKs: TypeScript resolves the wallet address first and then checksinclude, while Python checkedincludefirst. Sowithdrawals(include="")with no wallet address configured and noaddressoverride raisedMissingWalletAddressin TypeScript butValidationError("include must not be empty")in Python.This PR moves the wallet-address resolution in
sdks/python/pmxt/escrow.pyahead of theincludecheck, matching the TypeScript SDK (and Python's ownapprove_tx). Behaviour for valid inputs and the generated request path are unchanged. A newsdks/python/tests/test_escrow_withdrawals.pycovers the validation order (missing address + emptyinclude→MissingWalletAddress; resolved address + blankinclude→ValidationError(field="include")) and the request path that gets built.I only changed
withdrawalshere. The matchingdeposit_txdrift is tracked separately in #1892, and I didn't want to pre-empt how that one gets resolved. Address-first is what TypeScript does here and what Python'sapprove_txalready does, so this fix should line up with either outcome of #1892.Motivation / disclosure: I had some spare AI-assistant quota (Claude Code) and am using it to try to help projects with open good-first-issues. The change was prepared with Claude Code and verified as listed below. If it doesn't fit, isn't up to your bar, or you'd simply rather not take it — please feel free to close it, no hard feelings at all 🙂
Related issue
Closes #2353
Checklist
cd sdks/python && pytest→ 270 passed, 66 deselected (integration). The new testtest_missing_wallet_address_is_checked_before_empty_includefails without the fix and passes with it.black --checkpasses on the new test file.)CHANGELOG.mdis updated (if applicable) — n/a, root changelog appears to be written at release time; happy to add an entry if you prefer🤖 Generated with Claude Code