Sponsored Content
Skip to content

Retire fixTrustLinesToSelf - #5989

Merged
bthomee merged 2 commits into
developfrom
a1q123456/retire-fixTrustLinesToSelf
Nov 5, 2025
Merged

Retire fixTrustLinesToSelf#5989
bthomee merged 2 commits into
developfrom
a1q123456/retire-fixTrustLinesToSelf

Conversation

@a1q123456

@a1q123456 a1q123456 commented Nov 3, 2025

Copy link
Copy Markdown
Contributor

High Level Overview of Change

Context of Change

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Performance (increase or change in throughput and/or latency)
  • Tests (you added tests for code that already exists, or your new feature included in this PR)
  • Documentation update
  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
  • Release

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

@a1q123456

Copy link
Copy Markdown
Contributor Author

The code is here but we need to double check if there's no buggy trust lines so I'll update later.

@a1q123456

a1q123456 commented Nov 3, 2025

Copy link
Copy Markdown
Contributor Author
[
    {
        "Balance": {
            "currency": "KZC",
            "issuer": "rrrrrrrrrrrrrrrrrrrrBZbvji",
            "value": "-1.01"
        },
        "Flags": 131072,
        "HighLimit": {
            "currency": "KZC",
            "issuer": "r3cte32EE48W6muEUG2dFnfPmP2Q6bSgi3",
            "value": "0"
        },
        "HighNode": "0",
        "LedgerEntryType": "RippleState",
        "LowLimit": {
            "currency": "KZC",
            "issuer": "r3cte32EE48W6muEUG2dFnfPmP2Q6bSgi3",
            "value": "0"
        },
        "LowNode": "0",
        "PreviousTxnID": "79AAFF16B3AFDCB46724BB69EADD154F3524106F6F01FC4A8494E02904758DF9",
        "PreviousTxnLgrSeq": 639027,
        "index": "2F8F21EFCAFD7ACFB07D5BB04F0D2E18587820C7611305BB674A64EAB0FA71E1"
    },
    {
        "Balance": {
            "currency": "BYC",
            "issuer": "rrrrrrrrrrrrrrrrrrrrBZbvji",
            "value": "-1.414"
        },
        "Flags": 131072,
        "HighLimit": {
            "currency": "BYC",
            "issuer": "rBztYvjByTk1DsokHb28bGS2SAPJTkQd2A",
            "value": "0"
        },
        "HighNode": "0",
        "LedgerEntryType": "RippleState",
        "LowLimit": {
            "currency": "BYC",
            "issuer": "rBztYvjByTk1DsokHb28bGS2SAPJTkQd2A",
            "value": "0"
        },
        "LowNode": "0",
        "PreviousTxnID": "A2CAF83D4CFC2E2E65047C720FFE8FC81478E310C860B0FC23E9CF3296F7E8C1",
        "PreviousTxnLgrSeq": 639013,
        "index": "326035D5C0560A9DA8636545DD5A1B0DFCFF63E68D491B5522B767BB00564B1A"
    }
]

So there are 2 trust lines that the high account is the same as the low account, which means that we can't fully retire the feature at this moment, and we should at least keep the part that deletes such trust lines if we want to do it now but that doesn't look tidy - or, we could probably modify the Change transaction and delete those trust lines?

@a1q123456

Copy link
Copy Markdown
Contributor Author

Attaching my script here:

#!/usr/bin/env python3
"""
Fetch all RippleState trust lines from a given XRP Ledger ledger
and return only those where LowLimit.issuer == HighLimit.issuer.

Notes
- Uses JSON-RPC over HTTP POST to the server (e.g. Ripple's public Clio).
- Ignores any 'id' field from examples; not needed.
- Forces type = "RippleState" and binary = false.
- Handles both plain and 'FinalFields' wrapped entries.
- Streams results to a JSONL file to avoid high memory usage.

Example:
  python find_same_issuer_trustlines.py \
      --server https://s1.ripple.com:51234 \
      --ledger-hash 842B57C1CC0613299A686D3E9F310EC0422C84D3911E5056389AA7E5808A93C8 \
      --out matches.jsonl
"""
import argparse
import json
import sys
import time
from typing import Any, Dict, Iterable, Optional
import requests


def build_request_payload(
    ledger_hash: Optional[str],
    ledger_index: Optional[str],
    limit: int,
    marker: Optional[str],
) -> Dict[str, Any]:
    payload: Dict[str, Any] = {
        "method": "ledger_data",
        "params": [
            {
                "type": "RippleState",
                "binary": False,
                "limit": limit,
            }
        ],
    }
    p = payload["params"][0]
    if ledger_hash:
        p["ledger_hash"] = ledger_hash
    if ledger_index:
        p["ledger_index"] = ledger_index
    if marker:
        p["marker"] = marker
    return payload


def unwrap_fields(le: Dict[str, Any]) -> Dict[str, Any]:
    """
    ledger_data may return entries either as flat fields or under FinalFields.
    This normalises to the effective final fields.
    """
    if "FinalFields" in le and isinstance(le["FinalFields"], dict):
        return le["FinalFields"]
    return le


def same_issuer(le: Dict[str, Any]) -> bool:
    """
    Returns True if this RippleState entry has the same issuer on both sides.
    """
    f = unwrap_fields(le)
    low = f.get("LowLimit", {})
    high = f.get("HighLimit", {})
    try:
        return (
            isinstance(low, dict)
            and isinstance(high, dict)
            and low.get("issuer") is not None
            and high.get("issuer") is not None
            and str(low["issuer"]) == str(high["issuer"])
        )
    except Exception:
        return False


def fetch_pages(
    server: str,
    ledger_hash: Optional[str],
    ledger_index: Optional[str],
    limit: int,
    request_timeout: float,
    pause_between: float,
) -> Iterable[Dict[str, Any]]:
    """
    Iterates over all pages returned by ledger_data with type RippleState.
    Yields individual ledger entries.
    """
    marker: Optional[str] = None
    session = requests.Session()

    while True:
        payload = build_request_payload(ledger_hash, ledger_index, limit, marker)
        try:
            resp = session.post(server, json=payload, timeout=request_timeout)
        except requests.RequestException as e:
            raise SystemExit(f"HTTP error talking to {server}: {e}") from e

        if resp.status_code != 200:
            raise SystemExit(f"Non-200 status {resp.status_code}: {resp.text}")

        try:
            data = resp.json()
        except ValueError:
            raise SystemExit("Response was not valid JSON")

        result = data.get("result")
        if not isinstance(result, dict):
            raise SystemExit(f"Unexpected response shape: {json.dumps(data)[:500]}")

        state = result.get("state", [])
        if not isinstance(state, list):
            raise SystemExit(f"Unexpected 'state' shape: {json.dumps(result)[:500]}")

        for le in state:
            # Filter by LedgerEntryType just in case
            typ = le.get("LedgerEntryType") or unwrap_fields(le).get("LedgerEntryType")
            if typ == "RippleState":
                yield le

        marker = result.get("marker")
        if not marker:
            break

        if pause_between > 0:
            time.sleep(pause_between)


def main() -> None:
    ap = argparse.ArgumentParser(description="Find RippleState trust lines with same issuer on both sides.")
    ap.add_argument("--server", default="https://s1.ripple.com:51234",
                    help="JSON-RPC endpoint URL for rippled/Clio (default: Ripple public Clio)")
    g = ap.add_mutually_exclusive_group()
    g.add_argument("--ledger-hash", help="Specific ledger hash to scan")
    g.add_argument("--ledger-index", help="Specific ledger index to scan (e.g. 'validated' or a number)")
    ap.add_argument("--limit", type=int, default=200,
                    help="Page size for ledger_data requests (max server-dependent, default 200)")
    ap.add_argument("--out", default="matches.jsonl",
                    help="Output JSONL file for matching entries")
    ap.add_argument("--timeout", type=float, default=30.0,
                    help="HTTP request timeout seconds (default 30)")
    ap.add_argument("--pause", type=float, default=0.0,
                    help="Pause seconds between pages to be gentle (default 0)")
    args = ap.parse_args()

    if not args.ledger_hash and not args.ledger_index:
        print("Tip: you did not specify --ledger-hash or --ledger-index. Defaulting to 'validated'.", file=sys.stderr)
        args.ledger_index = "validated"

    total = 0
    matched = 0

    with open(args.out, "w", encoding="utf-8") as fh:
        for le in fetch_pages(
            server=args.server,
            ledger_hash=args.ledger_hash,
            ledger_index=args.ledger_index,
            limit=args.limit,
            request_timeout=args.timeout,
            pause_between=args.pause,
        ):
            total += 1
            if same_issuer(le):
                matched += 1
                fh.write(json.dumps(le, ensure_ascii=False) + "\n")

    print(f"Scanned RippleState entries: {total}")
    print(f"Matches where LowLimit.issuer == HighLimit.issuer: {matched}")
    print(f"Wrote matches to: {args.out}")


if __name__ == "__main__":
    main()

@ximinez

ximinez commented Nov 3, 2025

Copy link
Copy Markdown
Contributor

@a1q123456 Can you post just the ledger entry IDs of the objects your script found? They should have been impossible to create, so I'm wondering if the amendment actually failed to do what it was supposed to.

@a1q123456

a1q123456 commented Nov 4, 2025

Copy link
Copy Markdown
Contributor Author

@a1q123456 Can you post just the ledger entry IDs of the objects your script found? They should have been impossible to create, so I'm wondering if the amendment actually failed to do what it was supposed to.

I think I was checking against an old ledger, and they don't exist anymore now. The ledger entry hash are 326035D5C0560A9DA8636545DD5A1B0DFCFF63E68D491B5522B767BB00564B1A and 2F8F21EFCAFD7ACFB07D5BB04F0D2E18587820C7611305BB674A64EAB0FA71E1 and they exist on the 842B57C1CC0613299A686D3E9F310EC0422C84D3911E5056389AA7E5808A93C8 ledger

@a1q123456

Copy link
Copy Markdown
Contributor Author

Checked against the ledger F8A1E664EC9DDAE2922F798E2B4D6B02804502022EC24AC1AE1A94E8742AF9E9 and I don't see any trust lines that the two accounts are the same.

python test.py --ledger-hash F8A1E664EC9DDAE2922F798E2B4D6B02804502022EC24AC1AE1A94E8742AF9E9
Scanned RippleState entries: 4318
Matches where LowLimit.issuer == HighLimit.issuer: 0
Wrote matches to: matches.jsonl

Signed-off-by: JCW <a1q123456@users.noreply.github.com>

# Conflicts:
#	include/xrpl/protocol/detail/features.macro
@a1q123456
a1q123456 force-pushed the a1q123456/retire-fixTrustLinesToSelf branch from 974645d to bfe8df3 Compare November 4, 2025 13:58
@a1q123456
a1q123456 marked this pull request as ready for review November 4, 2025 13:59
@a1q123456
a1q123456 requested review from a team, pratikmankawde and vvysokikh1 November 4, 2025 13:59
@codecov

codecov Bot commented Nov 5, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.3%. Comparing base (f28ba57) to head (47d8200).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #5989     +/-   ##
=========================================
+ Coverage     78.2%   78.3%   +0.1%     
=========================================
  Files          816     816             
  Lines        68948   68887     -61     
  Branches      8352    8303     -49     
=========================================
- Hits         53950   53942      -8     
+ Misses       14998   14945     -53     
Files with missing lines Coverage Δ
src/xrpld/app/tx/detail/Change.cpp 85.9% <ø> (+15.9%) ⬆️
src/xrpld/app/tx/detail/Change.h 100.0% <ø> (ø)
src/xrpld/app/tx/detail/SetTrust.cpp 95.8% <100.0%> (+3.3%) ⬆️

... and 2 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@a1q123456 a1q123456 added the Needs additional review PR requires at least one more code review approval before it can be merged label Nov 5, 2025
@bthomee bthomee removed the Needs additional review PR requires at least one more code review approval before it can be merged label Nov 5, 2025
@a1q123456 a1q123456 added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label Nov 5, 2025
@bthomee
bthomee enabled auto-merge (squash) November 5, 2025 14:35
@bthomee
bthomee merged commit 673fb06 into develop Nov 5, 2025
3 checks passed
@bthomee
bthomee deleted the a1q123456/retire-fixTrustLinesToSelf branch November 5, 2025 14:56
ximinez added a commit that referenced this pull request Nov 6, 2025
…inez/lending-XLS-66

* mywork/ximinez/lending-number:
  Catch up the consequences of Number changes
  Fix build error - avoid copy
  Add integer enforcement when converting to XRP/MPTAmount to Number
  Make all STNumber fields "soeDEFAULT"
  Add optional enforcement of valid integer range to Number
  fix: domain order book insertion #5998
  refactor: Retire fixTrustLinesToSelf amendment (#5989)
@mvadari mvadari added this to the 3.2.0 milestone May 20, 2026
beartec-jpg pushed a commit to beartec-jpg/FalconLedger that referenced this pull request Jun 1, 2026
Amendments activated for more than 2 years can be retired. This change retires the fixTrustLinesToSelf amendment.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants