Functional one-time payments and tests by turukawa · Pull Request #12 · interledger/open-payments-python-sdk

turukawa · 2025-12-05T16:02:43Z

This is a fairly substantial pull request to get to a working version of the SDK. The updates can be divided into the following components:

Model refactoring and fixes: generated code resulted in models that don't return data, or duplications and deeply-nested RootModels much of which could be deleted and simplified.
New convenience functions: testing the API effectively requires building a full purchase workflow, but such a workflow is also useful to developers using the SDK. The README in the process folder details the main objectives and roadmap, and this update implements the simplest possible payments workflow for purchase and sale.
Updated tests: all the tests should be refactored, especially as private endpoints have been committed to the repo. Tests for the convenience functions run through a complete workflow, and use the new environment variables to safely do a real-world test on the Rafiki test server.

I have commented the code I've introduced, with references, and hope this is straightforward to read.

All tests green. To run your own tests, please review the .env.example and add your own values.

`http-sfv` is deprecated in favour of `http-sf`. Replaced accordingly.

`classmethod` must return the value holding class data.

Optional fields are optional and not always returned.

These take in a variety of fixes: - Row alignment and spacing - Correct dependency import path Note that this is only the most basic of fixes and much more needs to be done to fix duplicate functions and inefficient models.

It is good practice to place all environment variables - even demo ones used in 'test' - into a `.env` file that is excluded in the `.gitignore`. This commit adds: - `pydantic-settings` to import environment variables from a `.env` - `.env.example` for others to add their own settings - `config.py` to import the settings and make them available in tests.

There are numerous changes: - Removal of all unnecessary `RootModel` classes and refactoring for simple inheritence. - Removal of duplicated `WalletAddress` class from `resource.py` and `auth.py`. - Simplification of bizarre duplication of `LimitsOutgoing1`, `LimitsOutgoing2`, etc into a single class. And multiple other quality of life fixes and improves. I have no doubt this could be improved further, but this is significantly easier to navigate and read.

The parser is useful for: - Normalising wallet addresses from the form `$ilp.` to `https://ilp`. - Converting stored private keys back to PEM format. - Verifying payment response hash to validate buyer authentication.

This is the simplest function for processing a one-off payment between a purchaser and seller. Includes: - Pydantic models defining a seller account, and a pending incoming payment transaction that can be used to store stateless interactions (i.e. generate approval link -> third-party approval -> redirect for transaction completion) - One-time payment process workflow. A README covers the context and future functionality for this feature.

New dev dependencies required: `selenium` for authenticating purchase transactions via web, and `jupyterlab` for dev testing of code. the `.env` requires a test wallet login and password to test the purchase approval workflow.

johngian · 2026-03-07T09:56:54Z

Other than the code specific comments. I suggest you squash the 2 black/auto-formating related commits to one.

johngian · 2026-03-07T10:11:31Z

I added a bunch of comments. The model refactoring is not easily reviewable after all the changes so I am OK with merging it as it is. My only concern is that we diverge a lot from the OpenAPI specs, which is fine if that improves usability and the actual spec doesn't change much to be problematic. I suggest we move forward to have a working client and then nitpick the spec.

johngian · 2026-03-07T10:14:38Z

I also found some time to pick up some work and tried to test with real world data here: #14

There is some overlap on the base model changes (I only noticed your PR after my changes unfortunately). I have a cherry-picked version of my peer-to-peer transactions example with a bunch of fixes, but lets merge this one first.

Here is my working version of the end-to-end example using this PR as a base.
https://github.com/johngian/open-payments-python-sdk/tree/cherry-picked-peer-to-peer-with-pr12

johngian · 2026-03-07T10:43:56Z

Regarding the process implementation, overall i like the idea of abstracting it, but it feels very restrictive to a buyer/seller relationship. I would rather like to use the same abstraction but with languages that is more relevant to the existing docs to avoid confusion.

johngian · 2026-03-07T11:29:39Z

Heads up @Tymmmy @mkurapov, this review might be worth it to be prioritized because it brings the python support in a very much working state.

Tymmmy · 2026-03-11T09:04:54Z

@turukawa can you update the PR with the comments applied from @johngian and then I can aproove and merge the PR?

mkurapov · 2026-03-12T10:43:30Z

Thank you @johngian! Did you end up hitting submit on the review? I cannot see the code specific comments

turukawa · 2026-03-12T10:56:25Z

@Tymmmy @johngian @mkurapov Thanks, yes, will update (and I'm sure I'll have questions as I go). I'm busy with another project for the next week or so, but I'll get to this as quickly as I can.

johngian · 2026-03-07T09:49:13Z

        Compute Digest
        """
-        request.headers["Content-Digest"] = str(http_sfv.Dictionary({"sha-512": hashlib.sha512(request.content).digest()}))
+        request.headers["Content-Digest"] = ser({"sha-512": hashlib.sha512(request.content).digest()})


Since even the http_message_signatures lib doesn't use http_sf this, this can be simplified to one less dependency with:

digest = hashlib.sha512(request.content).digest() request.headers["Content-Digest"] = f"sha-512=:{base64.b64encode(digest).decode('ascii')}:"

johngian · 2026-03-07T09:50:35Z

        )
        response = request = self.sign_request(request,("authorization",*get_default_covered_components()))
        self.http_client.send(request=request)
+        req_headers = {**self.get_auth_header(access_token=access_token)}


The actual problem is a few lines above where response = request = <blah>. I guess this is an oversight of the initial bootstrapping codebase.

johngian · 2026-03-07T09:52:33Z

        extra="forbid",
    )
-    startCursor: Optional[constr(min_length=1)] = Field(
+    startCursor: Optional[str] = Field(


Not that this is a big problem, but this will validate even for empty string.

johngian · 2026-03-07T09:52:40Z

        description="Cursor corresponding to the first element in the result array.",
    )
-    endCursor: Optional[constr(min_length=1)] = Field(
+    endCursor: Optional[str] = Field(


johngian · 2026-03-07T09:55:18Z

-    )
+    createdAt: datetime = Field(..., description="The date and time when the outgoing payment was created.")
+    updatedAt: Optional[datetime] = Field(None, description="The date and time when the outgoing payment was updated.")
+    model_config = ConfigDict()


There are 2 model configs defined here. Check at the start of the class definition. Ideally I would prefer extra=forbid to be more strict on validation.

johngian · 2026-03-07T10:05:28Z

+        if isinstance(wallet_address, AnyUrl):
+            return str(wallet_address)
+        wallet_address = wallet_address.strip("$").strip("/")
+        if wallet_address.startswith("https://"):


Maybe you can use urlparse instead of assuming that its a valid address and using string manipulation, after removing the $ prefix.

johngian · 2026-03-07T10:38:43Z

+        except Exception:
+            return False
+
+    def convert_private_key_to_PEM(self, *, private_key: str | bytes, format: str = "PRIVATE KEY") -> str:


Since the private key is submitted on sdk call, instead of trying to guess what the format is, maybe you can be more explicit and require the format to be defined before hand. Then there is no need to parse the key, if its base64 or pem according to the format, if not internal cryptography is going to fail.

johngian · 2026-03-07T10:39:10Z

+            return wallet_address
+        return f"https://{wallet_address}"
+
+    def isBase64(self, *, term: str | bytes) -> bool:


Same here, i suggest we are explicit on the format instead of trying to identify it.

johngian · 2026-03-07T10:41:08Z

+        return value
+
+
+class SellerOpenPaymentAccount(BaseModel):


I don't think that seller is a good naming pattern, since we are not only dealing with sales.

johngian · 2026-03-07T10:42:14Z

+            http_client = HttpClient(http_timeout=10.0)
+        self.http_client = http_client
+        self.seller = seller
+        self.buyer = paymentsparser.normalise_wallet_address(wallet_address=buyer)


Same here, its not always a seller and a buyer, lets make it a bit more relevant to the overall API language.

turukawa added 13 commits November 20, 2025 10:28

Updated HTTP structured field dependency

5f226c3

`http-sfv` is deprecated in favour of `http-sf`. Replaced accordingly.

Pydantic classmethod fix

840080f

`classmethod` must return the value holding class data.

Fixed outgoing response model validation

d5fd01e

Fixed Python 2 class format

4e0c8a0

Fixed Pydantic field constraints

f6ba227

Fixed Pydantic OpenPayments schema validation

7138e17

Optional fields are optional and not always returned.

Python Black format conformance

371623a

These take in a variety of fixes: - Row alignment and spacing - Correct dependency import path Note that this is only the most basic of fixes and much more needs to be done to fix duplicate functions and inefficient models.

Refactor for Black standards

3cb0270

Convenience parser

88588c7

The parser is useful for: - Normalising wallet addresses from the form `$ilp.` to `https://ilp`. - Converting stored private keys back to PEM format. - Verifying payment response hash to validate buyer authentication.

Updated tests for convenience processes

f60c8f2

New dev dependencies required: `selenium` for authenticating purchase transactions via web, and `jupyterlab` for dev testing of code. the `.env` requires a test wallet login and password to test the purchase approval workflow.

johngian reviewed Mar 16, 2026

View reviewed changes

Conversation

turukawa commented Dec 5, 2025

Uh oh!

johngian commented Mar 7, 2026

Uh oh!

johngian commented Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johngian commented Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johngian commented Mar 7, 2026

Uh oh!

johngian commented Mar 7, 2026

Uh oh!

Tymmmy commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mkurapov commented Mar 12, 2026

Uh oh!

turukawa commented Mar 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

johngian commented Mar 7, 2026 •

edited

Loading

johngian commented Mar 7, 2026 •

edited

Loading

Tymmmy commented Mar 11, 2026 •

edited

Loading