Add marker parameter to get_account_transactions

[JST5000]

High Level Overview of Change

Add the optional parameter marker to get_account_transactions to allow pagination through results.

Context of Change

Resolves #426

Type of Change

• [X] New feature (non-breaking change which adds functionality)

Test Plan

Manual run of the code against testnet since the integration tests currently use a standalone node which doesn't have enough results to produce a marker.

[JST5000]

Here's the script I used to test the code:

import asyncio
from xrpl.asyncio.account.transaction_history import get_account_transactions
from xrpl.models.requests.account_tx import AccountTx
from xrpl.asyncio.clients.async_websocket_client import AsyncWebsocketClient


async def test_marker():
    client = AsyncWebsocketClient("wss://s.altnet.rippletest.net:51233")
    await client.open()
    testnetFaucet = "rPT1Sjq2YGrBMTttX4GZHjKu9dyfzbpAYe"
    request = AccountTx(account=testnetFaucet)
    response = await client.request_impl(request)
    transactions = await get_account_transactions(testnetFaucet, client, 
                    response.result.get("marker"))
    print(response.result["marker"])
    print(len(response.result["transactions"]), len(transactions))
    print(response.result["transactions"][-1], "\n\n\n", transactions[-1])
    await client.close()

asyncio.run(test_marker())

And the output (which showed it produce multiple results):

{'ledger': 30581332, 'seq': 2}
200 200
{'meta': {'AffectedNodes': [{'CreatedNode': {'LedgerEntryType': 'AccountRoot', 'LedgerIndex': '1403525C0ACBBEC6229DC7BB93F43FE8B12ECC6534897E2AAEE6F91531DCF858', 'NewFields': {'Account': 'rJHcCPJQttxoFu41brH96hmr6AJHZH1cKq', 'Balance': '1000000000', 'Sequence': 30581344}}}, {'ModifiedNode': {'FinalFields': {'Account': 'rPT1Sjq2YGrBMTttX4GZHjKu9dyfzbpAYe', 'Balance': '94833679087485796', 'Flags': 0, 'OwnerCount': 0, 'Sequence': 4161631}, 'LedgerEntryType': 'AccountRoot', 'LedgerIndex': '31CCE9D28412FF973E9AB6D0FA219BACF19687D9A2456A0C2ABC3280E9D47E37', 'PreviousFields': {'Balance': '94833680087485808', 'Sequence': 4161630}, 'PreviousTxnID': '25F228FEFDC1F3A220F1122BC608EDF51B361F8691E9C2CFD1D0898C5426B4CB', 'PreviousTxnLgrSeq': 30581332}}], 'TransactionIndex': 4, 'TransactionResult': 'tesSUCCESS', 'delivered_amount': '1000000000'}, 'tx': {'Account': 'rPT1Sjq2YGrBMTttX4GZHjKu9dyfzbpAYe', 'Amount': '1000000000', 'Destination': 'rJHcCPJQttxoFu41brH96hmr6AJHZH1cKq', 'Fee': '12', 'Flags': 2147483648, 'LastLedgerSequence': 30581346, 'Sequence': 4161630, 'SigningPubKey': '02356E89059A75438887F9FEE2056A2890DB82A68353BE9C0C0C8F89C0018B37FC', 'TransactionType': 'Payment', 'TxnSignature': '304402205F08688B533E2AB03076EE7A727CC1DF765015CC000343E93B24E2620CFC6C4002204179FC38710511F7EFA2E919C347985E6F64C57E39D01592552B7A0F2A12046F', 'date': 714610470, 'hash': '617548A0C52F18BA7A0735DA0C934057B5EE89A6A1F3F6C33B774E61AF2BBE12', 'inLedger': 30581344, 'ledger_index': 30581344}, 'validated': True} 


 {'meta': {'AffectedNodes': [{'CreatedNode': {'LedgerEntryType': 'AccountRoot', 'LedgerIndex': '270ACDF426444441659083BF9B9E24BF613B4E9A42469AFE6A79D24740E689BA', 'NewFields': {'Account': 'r9JdnervKskcGboWnrS8KFEq4fcr6SeU7d', 'Balance': '1000000000', 'Sequence': 30580233}}}, {'ModifiedNode': {'FinalFields': {'Account': 'rPT1Sjq2YGrBMTttX4GZHjKu9dyfzbpAYe', 'Balance': '94833879087488196', 'Flags': 0, 'OwnerCount': 0, 'Sequence': 4161431}, 'LedgerEntryType': 'AccountRoot', 'LedgerIndex': '31CCE9D28412FF973E9AB6D0FA219BACF19687D9A2456A0C2ABC3280E9D47E37', 'PreviousFields': {'Balance': '94833880087488208', 'Sequence': 4161430}, 'PreviousTxnID': '03E9FBDA12DCAC6AA59C09F1EBD382212F0E998B35A1FD54318DC92D2F7A033F', 'PreviousTxnLgrSeq': 30580207}}], 'TransactionIndex': 1, 'TransactionResult': 'tesSUCCESS', 'delivered_amount': '1000000000'}, 'tx': {'Account': 'rPT1Sjq2YGrBMTttX4GZHjKu9dyfzbpAYe', 'Amount': '1000000000', 'Destination': 'r9JdnervKskcGboWnrS8KFEq4fcr6SeU7d', 'Fee': '12', 'Flags': 2147483648, 'LastLedgerSequence': 30580236, 'Sequence': 4161430, 'SigningPubKey': '02356E89059A75438887F9FEE2056A2890DB82A68353BE9C0C0C8F89C0018B37FC', 'TransactionType': 'Payment', 'TxnSignature': '30440220273B33D0CFA0C62907EC3E7EDCD3493CDFE13B8C813C13B43156D1F97C2171DC0220715440909D894C5E7627A5D5D5CBD6F2ADB3FC247511611C8089929D8883A089', 'date': 714607000, 'hash': '02CC7C056DC39B31768367E5B58E15149CAC8B562012EFDB91439F216861A942', 'inLedger': 30580233, 'ledger_index': 30580233}, 'validated': True}

[ckniffen]

Would a unit test suffice to make sure it is passed through to the underlying methods?

[JST5000]

Would a unit test suffice to make sure it is passed through to the underlying methods?

@ckniffen I'm not really sure how to set up a unit test that would do that with the xrpl-py setup. I don't think it would add much protection either, as if future changes break the code, it'll likely break the return instead of whether the function receives parameters.

@mvadari do you happen to know any examples of unit tests within the repo which have a similarly "rare" condition to test that I could look up as examples of how to test something like a "marker" appearing/being used?

[JST5000]

Separately, @LimpidCrypto with this implementation I wrote, the function does NOT return the marker, which is a bit of a usability issue. (Because it doesn't allow for continuous pagination).

Directly using the request you can do normal pagination by retrieving the marker yourself (as I did in the test script above), but it's not exactly an easier process.

Would this implementation work for the problem you were describing, or is it necessary to also retrieve the marker after each call? (In which case I'm thinking of creating a separate function to avoid breaking changes)

[LimpidCrypto]

Separately, @LimpidCrypto with this implementation I wrote, the function does NOT return the marker, which is a bit of a usability issue. (Because it doesn't allow for continuous pagination).

Directly using the request you can do normal pagination by retrieving the marker yourself (as I did in the test script above), but it's not exactly an easier process.

Would this implementation work for the problem you were describing, or is it necessary to also retrieve the marker after each call? (In which case I'm thinking of creating a separate function to avoid breaking changes)

As you already said it would not really make it easier. If the user works with get_account_transactions, the user doesn‘t have a quick way to check wether all transactions for that account were received. To check if a marker has been returned would be way easier. IMO we should have a method that also returns the marker if provided by the node :)

[JST5000]

If the user works with get_account_transactions, the user doesn‘t have a quick way to check wether all transactions for that account were received. To check if a marker has been returned would be way easier.

Ok, I'll make a function called get_account_transactions_with_marker that returns and allows for the marker. Then I think deprecating get_account_transactions in favor of the new function would make sense. (That way we can just have that be the default behavior going forward, since it's a pretty important part of that functionality to be able to tell if there are more results)

If there's any considerations I'm missing which make that plan not ideal, please let me know :)

[JST5000]

(Still need to export these new functions, which I've been struggling to do because it seems like Sphinx isn't working - so the pre-commit hook for Sphinx is blocking changes. Trying to figure out why that is)

[JST5000]

@mvadari pointed out that this with_marker variant was basically the same thing as calling AccountTx directly, so I just added to the docs pointing out that if you need the marker you can directly make the request. No point adding an extra name for something that's already pretty much a one-line implementation.

[JST5000]

(This PR still is also required to fix the Sphinx dependency issues which were preventing merges previously - updated the PR title to reflect that)