Skip to content

Conversation

maria-robobug
Copy link
Contributor

@maria-robobug maria-robobug commented Sep 12, 2025

@maria-robobug maria-robobug force-pushed the python-get-started-code-walkthrough branch from bda1da2 to aa74ce3 Compare September 12, 2025 17:11
@maria-robobug maria-robobug force-pushed the python-get-started-code-walkthrough branch from aa74ce3 to a8597a8 Compare September 12, 2025 17:22
@maria-robobug maria-robobug marked this pull request as ready for review September 12, 2025 17:23
Copy link
Collaborator

@mDuo13 mDuo13 left a comment

Choose a reason for hiding this comment

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

The big picture looks good and I confirmed that you can download and run the code sample, with the expected results, but I noticed a few inaccuracies that should be corrected.

{% step id="connect-tag" %}
#### Connect to the XRP Ledger Testnet

To make queries and submit transactions, you need to connect to the XRP Ledger. To do this with `xrpl-py`, use the [`xrp.clients` module](https://xrpl-py.readthedocs.io/en/latest/source/xrpl.clients.html):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This line needs to change. It doesn't make sense in context to end the paragraph with a colon anymore.

2025-10-03_112507_622624610

Copy link
Collaborator

Choose a reason for hiding this comment

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

It might actually make sense to note here that the default with xrpl-py is to use JSON-RPC, not WebSocket (because writing async code in Python is annoying and kind of tends to make you need to do everything async) but there is an option for a WebSocket client if you need that functionality.


JSON_RPC_URL = "https://s.altnet.rippletest.net:51234/"
client = JsonRpcClient(JSON_RPC_URL)
print("Connected to Testnet")
Copy link
Collaborator

Choose a reason for hiding this comment

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

This print statement is inaccurate with how JSON-RPC works and potentially misleading in case of failure.

I assume you're copying from the JS version, but having a print statement in the equivalent place only works because the JS example uses WebSocket, which has a specific flow:

  1. Create a WS Client instance
  2. Establish a persistent connection to the network; if it fails, it'll throw an error
  3. Print that you're connected

Since the Python example uses JSON-RPC, you don't have a persistent connection; the client connects separately each time you make an API call. Because of that, nothing happens when you instantiate the client with a particular URL: it won't throw an error here if you pass a URL that isn't a real, online JSON-RPC server.

So, by printing "Connected to Testnet" here, you're potentially misleading people into thinking that the connection actually works, when nothing has even tried connecting to anything yet. Which means if there's a problem with the Testnet servers—or the user changed the URL to one that won't work—it'll confusingly and inaccurately say that it connected even though it didn't, which will complicate the debugging process.

Comment on lines +121 to +127
{% step id="get-account-x-address-tag" %}
#### Derive an X-Address


##### Sign and submit

To sign and submit the transaction:

{% code-snippet file="/_code-samples/get-started/py/prepare-payment.py" from="# Sign and submit the transaction" before="# Print tx response" language="py" /%}


##### Derive an X-address

You can use `xrpl-py`'s [`xrpl.core.addresscodec`](https://xrpl-py.readthedocs.io/en/latest/source/xrpl.core.addresscodec.html) module to derive an [X-address](https://xrpaddress.info/) from the `Wallet.address` field:

{% code-snippet file="/_code-samples/get-started/py/get-acct-info.py" from="# Derive an x-address from the classic address:" before="# Look up info about your account" language="py" /%}
You can use `xrpl-py`'s [`xrpl.core.addresscodec`](https://xrpl-py.readthedocs.io/en/latest/source/xrpl.core.addresscodec.html) module to derive an [X-address](https://xrpaddress.info/) from the `Wallet.address` field.

The X-address format [packs the address and destination tag](https://github.com/XRPLF/XRPL-Standards/issues/6) into a more user-friendly value.
{% /step %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hot take: we should consider removing this from the "Get Started" tutorial; I rarely see X-addresses being used in practice.


### 3. Query the XRP Ledger

{% step id="query-xrpl-tag" %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't it be more appropriate to mark each step such that it includes the header? (in this case, "4. Query the XRP Ledger") As it is, when someone clicks the text area around the step, the page scrolls so that the relevant header is just out of sight, which seems less than ideal.

2025-10-03_114508_283427144

{% code-snippet file="/_code-samples/get-started/py/get-acct-info.py" from="# Look up info about your account" language="py" /%}


Here, we use `xrpl-py`'s [`xrpl.account`](https://xrpl-py.readthedocs.io/en/latest/source/xrpl.account.html) module to look up information about the account we got in the previous step.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The sample code doesn't actually use the xrpl.account module anymore (it's not even imported!) so this sentence is out of date. It actually makes an account_info request using the request model to validate the format.

Also, per Microsoft style, we should use second person, not first person, for instructions. So this would more appropriately be written something like:

Suggested change
Here, we use `xrpl-py`'s [`xrpl.account`](https://xrpl-py.readthedocs.io/en/latest/source/xrpl.account.html) module to look up information about the account we got in the previous step.
Use the [account_info method][] to look up information about the account you got in the previous step. Use a request model like `AccountInfo` to validate the request format and catch errors sooner.


```sh
Classic address:
Connected to Testnet
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remember to update this (per my note earlier about how this print statement is misleading)

The response fields that you want to inspect in most cases are:

* `account_data.Sequence` — This is the sequence number of the next valid transaction for the account. You need to specify the sequence number when you prepare transactions. With `xrpl-py`, you can use the [`get_next_valid_seq_number`](https://xrpl-py.readthedocs.io/en/latest/source/xrpl.account.html#xrpl.account.get_next_valid_seq_number) to get this automatically from the XRP Ledger. See an example of this usage in the project [README](https://github.com/XRPLF/xrpl-py#serialize-and-sign-transactions).
- `account_data.Sequence` — This is the sequence number of the next valid transaction for the account. You need to specify the sequence number when you prepare transactions. With `xrpl-py`, you can use the [`get_next_valid_seq_number`](https://xrpl-py.readthedocs.io/en/latest/source/xrpl.account.html#xrpl.account.get_next_valid_seq_number) to get this automatically from the XRP Ledger. See an example of this usage in the project [README](https://github.com/XRPLF/xrpl-py#serialize-and-sign-transactions).
Copy link
Collaborator

@mDuo13 mDuo13 Oct 3, 2025

Choose a reason for hiding this comment

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

I'm not sure this bullet point even needs to be here, because manually specifying the sequence number is usually not necessary. You do occasionally need to do it—for example, if you're preparing a transaction on an air-gapped, offline machine, or if you're bulk submitting more than one transaction per few seconds—but those use cases are the exception, not the rule, so maybe we can actually just delete this entire bullet point to keep things simple.

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