Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add optional CTID field to Tx #2477

Merged
merged 7 commits into from
Nov 30, 2023
Merged

feat: Add optional CTID field to Tx #2477

merged 7 commits into from
Nov 30, 2023

Conversation

JST5000
Copy link
Contributor

@JST5000 JST5000 commented Sep 13, 2023

High Level Overview of Change

Add a field for the Concise Transaction ID that was added in rippled 1.12

Fixes #2409

Context of Change

Added to rippled via: XRPLF/rippled#4418

Type of Change

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

Did you update HISTORY.md?

  • Yes

Test Plan

CI Passes

@JST5000
Copy link
Contributor Author

JST5000 commented Sep 13, 2023

It seems like this change does not impact account_tx OR subscribing to transactions as far as I can tell by testing on Testnet.

If you run this tx command using the websocket api tool you'll see the ctid

Request:

{
  "id": 1,
  "command": "tx",
  "transaction": "8A66F7307EE3BA907986B4DE8F77DE35EAF972AE9285FDC7F7EBF3E061DB1D27",
  "binary": false
}

Response: (Contains ctid)

{
  "id": 1,
  "result": {
    "Account": "ruG4974Rjs6gMVzoPqVtHJShcbraHYXZU",
    "Amount": "20000000",
    "Destination": "rMVUdQ1m4xfdvWPxx2KTG83U86cYCR5a65",
    "Fee": "10",
    "Flags": 2147483648,
    "Sequence": 41162044,
    "SigningPubKey": "EDE1A6065C3C9B35D05EEB16F0E9B7D61838317891BC5A9E3E7A40A9D34851D6FB",
    "TransactionType": "Payment",
    "TxnSignature": "9F5E9F29B90AA88B6D2C5CCFF94DAA4278078754D556329709BB81CE4CA5E1E4359FEF1175AE7CC572B3265CA683A2F4CA9B7ECFC67CDE57B45742C6ED97370A",
    "ctid": "C274159900000001",
    "date": 747877990,
    "hash": "8A66F7307EE3BA907986B4DE8F77DE35EAF972AE9285FDC7F7EBF3E061DB1D27",
    "inLedger": 41162137,
    "ledger_index": 41162137,
    "meta": {
      "AffectedNodes": [
        {
          "ModifiedNode": {
            "FinalFields": {
              "Account": "ruG4974Rjs6gMVzoPqVtHJShcbraHYXZU",
              "Balance": "479999990",
              "Flags": 0,
              "OwnerCount": 0,
              "Sequence": 41162045
            },
            "LedgerEntryType": "AccountRoot",
            "LedgerIndex": "508AD608D29994BAB9BF2160D7A6F690485C990F7B9741E5BDBEC95432E7A6AA",
            "PreviousFields": {
              "Balance": "500000000",
              "Sequence": 41162044
            },
            "PreviousTxnID": "D6D03D10CCB28221DCFF7E3EB800F1D48414EC964DA8CE5DBA6012F159FFDA7B",
            "PreviousTxnLgrSeq": 41162044
          }
        },
        {
          "ModifiedNode": {
            "FinalFields": {
              "Account": "rMVUdQ1m4xfdvWPxx2KTG83U86cYCR5a65",
              "Balance": "57080000000",
              "Flags": 0,
              "OwnerCount": 0,
              "Sequence": 29435947
            },
            "LedgerEntryType": "AccountRoot",
            "LedgerIndex": "FB0D252DDC3170C499442EECA27D1776D8D4302CD5F6FF68DD0767A8434C291A",
            "PreviousFields": {
              "Balance": "57060000000"
            },
            "PreviousTxnID": "0F634CAD44DBE6388922821396774436ACB27087FF5A8BDCDC5E1ABE1B32A664",
            "PreviousTxnLgrSeq": 41158757
          }
        }
      ],
      "TransactionIndex": 0,
      "TransactionResult": "tesSUCCESS",
      "delivered_amount": "20000000"
    },
    "validated": true
  },
  "status": "success",
  "type": "response"
}

But if you run this account_tx command on the account which sent that transaction, there are no ctids

Request:

{
  "id": 2,
  "command": "account_tx",
  "account": "ruG4974Rjs6gMVzoPqVtHJShcbraHYXZU",
  "ledger_index_min": -1,
  "ledger_index_max": -1,
  "binary": false,
  "limit": 2,
  "forward": false
}

Response: (No ctids)

{
  "id": 2,
  "result": {
    "account": "ruG4974Rjs6gMVzoPqVtHJShcbraHYXZU",
    "ledger_index_max": 41162388,
    "ledger_index_min": 40435141,
    "limit": 2,
    "transactions": [
      {
        "meta": {
          "AffectedNodes": [
            {
              "ModifiedNode": {
                "FinalFields": {
                  "Account": "ruG4974Rjs6gMVzoPqVtHJShcbraHYXZU",
                  "Balance": "479999990",
                  "Flags": 0,
                  "OwnerCount": 0,
                  "Sequence": 41162045
                },
                "LedgerEntryType": "AccountRoot",
                "LedgerIndex": "508AD608D29994BAB9BF2160D7A6F690485C990F7B9741E5BDBEC95432E7A6AA",
                "PreviousFields": {
                  "Balance": "500000000",
                  "Sequence": 41162044
                },
                "PreviousTxnID": "D6D03D10CCB28221DCFF7E3EB800F1D48414EC964DA8CE5DBA6012F159FFDA7B",
                "PreviousTxnLgrSeq": 41162044
              }
            },
            {
              "ModifiedNode": {
                "FinalFields": {
                  "Account": "rMVUdQ1m4xfdvWPxx2KTG83U86cYCR5a65",
                  "Balance": "57080000000",
                  "Flags": 0,
                  "OwnerCount": 0,
                  "Sequence": 29435947
                },
                "LedgerEntryType": "AccountRoot",
                "LedgerIndex": "FB0D252DDC3170C499442EECA27D1776D8D4302CD5F6FF68DD0767A8434C291A",
                "PreviousFields": {
                  "Balance": "57060000000"
                },
                "PreviousTxnID": "0F634CAD44DBE6388922821396774436ACB27087FF5A8BDCDC5E1ABE1B32A664",
                "PreviousTxnLgrSeq": 41158757
              }
            }
          ],
          "TransactionIndex": 0,
          "TransactionResult": "tesSUCCESS",
          "delivered_amount": "20000000"
        },
        "tx": {
          "Account": "ruG4974Rjs6gMVzoPqVtHJShcbraHYXZU",
          "Amount": "20000000",
          "Destination": "rMVUdQ1m4xfdvWPxx2KTG83U86cYCR5a65",
          "Fee": "10",
          "Flags": 2147483648,
          "Sequence": 41162044,
          "SigningPubKey": "EDE1A6065C3C9B35D05EEB16F0E9B7D61838317891BC5A9E3E7A40A9D34851D6FB",
          "TransactionType": "Payment",
          "TxnSignature": "9F5E9F29B90AA88B6D2C5CCFF94DAA4278078754D556329709BB81CE4CA5E1E4359FEF1175AE7CC572B3265CA683A2F4CA9B7ECFC67CDE57B45742C6ED97370A",
          "date": 747877990,
          "hash": "8A66F7307EE3BA907986B4DE8F77DE35EAF972AE9285FDC7F7EBF3E061DB1D27",
          "inLedger": 41162137,
          "ledger_index": 41162137
        },
        "validated": true
      },
      {
        "meta": {
          "AffectedNodes": [
            {
              "CreatedNode": {
                "LedgerEntryType": "AccountRoot",
                "LedgerIndex": "508AD608D29994BAB9BF2160D7A6F690485C990F7B9741E5BDBEC95432E7A6AA",
                "NewFields": {
                  "Account": "ruG4974Rjs6gMVzoPqVtHJShcbraHYXZU",
                  "Balance": "500000000",
                  "Sequence": 41162044
                }
              }
            },
            {
              "ModifiedNode": {
                "FinalFields": {
                  "Account": "raNZBZyPRXj35zGixSdhv8Q61YhoBvhyEi",
                  "Balance": "9499999990",
                  "Flags": 0,
                  "OwnerCount": 0,
                  "Sequence": 41162042
                },
                "LedgerEntryType": "AccountRoot",
                "LedgerIndex": "8CF193D14C753A20D42695BA81D1260BD6C885D3B03270CF5F1DE8393027A71D",
                "PreviousFields": {
                  "Balance": "10000000000",
                  "Sequence": 41162041
                },
                "PreviousTxnID": "E3BDF0DDC0D83F86155771A66998D7ACB9A915B14F2202E9A1781C4C96C7A6BC",
                "PreviousTxnLgrSeq": 41162041
              }
            }
          ],
          "TransactionIndex": 3,
          "TransactionResult": "tesSUCCESS",
          "delivered_amount": "500000000"
        },
        "tx": {
          "Account": "raNZBZyPRXj35zGixSdhv8Q61YhoBvhyEi",
          "Amount": "500000000",
          "Destination": "ruG4974Rjs6gMVzoPqVtHJShcbraHYXZU",
          "Fee": "10",
          "Flags": 2147483648,
          "Sequence": 41162041,
          "SigningPubKey": "026919194A5E62C422647DB750FCDB45DC5214BC4B74BCC3379348BEF376113F56",
          "TransactionType": "Payment",
          "TxnSignature": "3044022059BA8B4390242B8CAFECE191020F9279E78106E66055C23DEC47B13139D7362D022020E0AC99321B026DEF7BB0611F26C7C0905F593CD8C6DC81FB143030F03AB579",
          "date": 747877700,
          "hash": "D6D03D10CCB28221DCFF7E3EB800F1D48414EC964DA8CE5DBA6012F159FFDA7B",
          "inLedger": 41162044,
          "ledger_index": 41162044
        },
        "validated": true
      }
    ],
    "validated": true
  },
  "status": "success",
  "type": "response"
}

As for the transactions stream - if you run this websocket command:

{
  "id": "Example watch one account and all new ledgers",
  "command": "subscribe",
  "streams": [
    "transactions"
  ]
}

Then pause the stream and search all results for ctid, you'll find nothing.

@intelliot do you know if that is expected behavior?

Comment on lines +14 to +21
/**
* The transaction hash to look up. Exactly one of `transaction` or `ctid` must be specified for a TxRequest.
*/
transaction?: string
/**
* The Concise Transaction ID to look up. Exactly one of `transaction` or `ctid` must be specified for a TxRequest.
*/
ctid?: string
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be a more specific type to ensure that one of the two is provided.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I saw when looking into this, I couldn't find a good way to ensure that one of the two was provided in a backwards compatible way.

The normal way "one of" is written is by combining multiple discrete types into a type alias, but then you can't initialize it like we do in snippets / a lot of code with Tx(...). So I don't think that path is worthwhile.

That problem is the same one with doing typing with meta types like this: https://stackoverflow.com/questions/40510611/typescript-interface-require-one-of-two-properties-to-exist

Any thoughts on how to ensure only one of the two is specified?

@intelliot
Copy link
Collaborator

@intelliot do you know if that is expected behavior?

That's a good catch, and I would consider it to be a bug. Can you check if XRPLF/rippled#4738 fixes that?

@JST5000
Copy link
Contributor Author

JST5000 commented Nov 29, 2023

Going to merge & create a follow up issue to look at the response types for requests and add CTID to them :)

@JST5000 JST5000 merged commit 98abafb into main Nov 30, 2023
18 checks passed
@JST5000 JST5000 deleted the ctid branch November 30, 2023 15:23
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.

Add CTID support
5 participants