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

A trust line with only lsf*Auth flags set is removed from the ledger [rippled version 1.12.0-rc4] #4698

Open
scottschurr opened this issue Sep 11, 2023 · 12 comments
Labels
Amendment Bug Good First Issue Great issue for a new contributor

Comments

@scottschurr
Copy link
Collaborator

Issue Description

A trust line is removed from the ledger unexpectedly.

Steps to Reproduce

The following unit test member function reproduces the problem.

    void
    testTrustLineWithOnlySetfAuthIsRemoved()
    {
        testcase("Trust line with only lsf*Auth");
        using namespace jtx;

        Env env{*this};

        // auth1 and auth2 are both issuers that require authorization.  They
        // will create a (required) trust line so they can cross offers between
        // themselves.
        Account const auth1("auth1");
        Account const auth2("auth2");
        auto const USD1 = auth1["USD"];
        auto const USD2 = auth2["USD"];

        env.fund(XRP(100000), auth1, auth2);
        env.close();
        env.require(owners(auth1, 0), owners(auth2, 0));

        // Trust lines to auth issuers must be authorized.
        env(fset(auth1, asfRequireAuth));
        env(fset(auth2, asfRequireAuth));
        env.close();

        // Since auth1 and auth2 have the asfRequireAuth flag set in their
        // accounts, they must explicitly give permission to holders of
        // their currencies.  This call creates a trust line with one of the
        // lsf*Auth flags set.
        env(trust(auth1, USD2(0), tfSetfAuth));
        env.close();
        // After creating the trust line we find this ledger entry as expected.
        BEAST_EXPECT(env.le(
            keylet::line(auth1.id(), auth2.id(), USD1.currency)));
        env.require(owners(auth1, 1), owners(auth2, 0));

        // Now auth2 reciprocates by attempting to set the other lsf*Auth
        // flag in the trust line.  Surprisingly, the trust line is removed
        // instead.
        env(trust(auth2, USD1(0), tfSetfAuth));
        env.close();
        // There's no good reason why the immediately preceeding TrustSet
        // should cause the previously inserted trust line to be removed
        // from the ledger.  But that's what happens.  So the following
        // tests fail.
        BEAST_EXPECT(env.le(
            keylet::line(auth1.id(), auth2.id(), USD1.currency)));
        env.require(owners(auth1, 1), owners(auth2, 1));
    }

The workaround is easy. Setting a non-zero trust limit changes the behaviors. Setting non-zero trust limits on both sides of the trust line results in expected behavior.

Given that there's an easy work around, use non-zero trust limits, this is not a high priority bug.

Environment

macOS Monterey 12.5.1
Apple clang version 13.1.6 (clang-1316.0.21.2.5)

@intelliot intelliot added Good First Issue Great issue for a new contributor Bug Amendment labels Sep 16, 2023
@ckeshava
Copy link
Collaborator

ckeshava commented Dec 7, 2023

Hello @scottschurr ,
Thank you for writing the unit test, it is helpful to visualize the context.

Is this not the intended behavior? I'm of the understanding that Trust Lines in their default settings are candidates for removal, irrespective of their Authorized flag status. (Docs: https://xrpl.org/trust-lines-and-issuing.html#reserves-and-deletion) But I'm not sure when this removal is triggered.

If we modify this behavior, we will have to rethink the conditions for deleting a Trust Line.

image

@scottschurr
Copy link
Collaborator Author

It looks like the behavior I'm seeing is documented and expected. So it makes sense to close this issue. Thanks for researching this and sorry for the noise.

@mvadari
Copy link
Collaborator

mvadari commented Dec 7, 2023

Hello @scottschurr , Thank you for writing the unit test, it is helpful to visualize the context.

Is this not the intended behavior? I'm of the understanding that Trust Lines in their default settings are candidates for removal, irrespective of their Authorized flag status. (Docs: https://xrpl.org/trust-lines-and-issuing.html#reserves-and-deletion)

Having the "Authorized" flag means that the trustline isn't in default settings. I've tried deleting trustlines with lsfAuth before, it doesn't work normally.

@mvadari
Copy link
Collaborator

mvadari commented Dec 7, 2023

Here's the script I wrote to test out this authorized trustline functionality, if anyone wants to replicate:

const xrpl = require("xrpl");

async function test() {
  const client = new xrpl.Client("wss://s.altnet.rippletest.net:51233");
  await client.connect();
  console.log("connected");
  const { wallet: issuer } = await client.fundWallet();
  const { wallet: customer } = await client.fundWallet();

  const accountSet = {
    TransactionType: 'AccountSet',
    Account: issuer.address,
    SetFlag: xrpl.AccountSetAsfFlags.asfRequireAuth,
  }
  console.log(await client.submitAndWait(accountSet, {autofill: true, wallet: issuer}))

  const customerTrust = {
    TransactionType: 'TrustSet',
    Account: customer.address,
    LimitAmount: {currency: "USD", issuer: issuer.address, value: "2"}
  }
  console.log(await client.submitAndWait(customerTrust, {autofill: true, wallet: customer}))

  const issuerTrust = {
    TransactionType: 'TrustSet',
    Account: issuer.address,
    LimitAmount: {currency: "USD", issuer: customer.address, value: "0"},
    Flags: xrpl.TrustSetFlags.tfSetfAuth,
  }
  console.log(await client.submitAndWait(issuerTrust, {autofill: true, wallet: issuer}))

  console.log((await client.request({
    command: 'account_objects',
    account: issuer.address,
  })).result.account_objects)

  console.log((await client.request({
    command: 'account_objects',
    account: customer.address,
  })).result.account_objects)

  const issuerDelete = {
    TransactionType: 'TrustSet',
    Account: issuer.address,
    LimitAmount: {currency: "USD", issuer: customer.address, value: "0"},
    Flags: 0,
  }
  console.log(await client.submitAndWait(issuerDelete, {autofill: true, wallet: issuer}))

  console.log((await client.request({
    command: 'account_objects',
    account: issuer.address,
  })).result.account_objects)

  console.log((await client.request({
    command: 'account_objects',
    account: customer.address,
  })).result.account_objects)

  await client.disconnect()
}

test()

@ckeshava
Copy link
Collaborator

ckeshava commented Dec 7, 2023

I'm not able to run this script, perhaps I'm missing a dependency

$ python3 trustlineDel.py # I have used this file-name for your script.
  File "/Users/ckeshavabs/personal-code/rippled/cmake-build-debug/trustlineDel.py", line 1
    const xrpl = require("xrpl");
          ^^^^
SyntaxError: invalid syntax

The expected behavior of your code is: The issuer's account should not have any objects whereas the customer's account should have one trust line. Is this correct?

Since the customer has not reset the trust line settings to default, the trust line will still take up one owner reserve.

@mvadari
Copy link
Collaborator

mvadari commented Dec 7, 2023

It's Javascript, not Python.

The expected behavior is that the trustline is created + authorized, but trying to delete the trustline by resetting everything else to default settings (since you can't unset lsfAuth) doesn't work - the trustline still exists.

@ckeshava
Copy link
Collaborator

ckeshava commented Dec 8, 2023

@mvadari Haha thanks for the clarification, my bad. Do you execute this with node.js runtime?

The customer has not reset the settings to default from their end. So the trust line should not be deleted. Doesn't the highlighted portion of the docs address your concern?
image

@mvadari
Copy link
Collaborator

mvadari commented Dec 8, 2023

@mvadari Haha thanks for the clarification, my bad. Do you execute this with node.js runtime?

Yes.

The customer has not reset the settings to default from their end. So the trust line should not be deleted. Doesn't the highlighted portion of the docs address your concern?

See the paragraph under the highlighted part.

@ckeshava
Copy link
Collaborator

ckeshava commented Dec 8, 2023

Here's a unit-test in C++ that seems to contradict your findings. It's a slight modification of @scottschurr 's code above. I've tried to mimic your javascript code in that unit test.

In your code, you haven't reset the customer's limit to zero, do you intend to keep it that way? It should be reset to 0, if you want to delete that trust line.

Nonetheless, I observed something strange with the javascript code. The persistence of the trust lines occurs even without the use of Authorized flag.

1. Create a trust line with non-zero limit, do not use Authorised flag
2. Reset the trust line's limit to zero. Send the TrustSet transaction.
3. Check the account_objects on both the accounts --> The trust line is not deleted

JS Code:

const xrpl = require("xrpl");

async function test() {
  const client = new xrpl.Client("wss://s.altnet.rippletest.net:51233");
  await client.connect();
  console.log("connected");
  const { wallet: issuer } = await client.fundWallet();
  const { wallet: customer } = await client.fundWallet();

  console.log("Creating trust line from customer's end\n")
  const customerTrust = {
    TransactionType: 'TrustSet',
    Account: customer.address,
    LimitAmount: {currency: "USD", issuer: issuer.address, value: "2"}
  }
  console.log(await client.submitAndWait(customerTrust, {autofill: true, wallet: customer}))

  console.log("Creating trust line from issuer's end\n")
  const issuerTrust = {
    TransactionType: 'TrustSet',
    Account: issuer.address,
    LimitAmount: {currency: "USD", issuer: customer.address, value: "0"},
  }
  console.log(await client.submitAndWait(issuerTrust, {autofill: true, wallet: issuer}))

  const custDelete = {
    TransactionType: 'TrustSet',
    Account: customer.address,
    LimitAmount: {currency: "USD", issuer: issuer.address, value: "0"},
  }
  console.log(await client.submitAndWait(custDelete, {autofill: true, wallet: customer}))

  console.log("Completed the reset of trustline from customer's" +
      " perspective. Account objects for issuer and customer resp\n")
  console.log((await client.request({
    command: 'account_objects',
    account: issuer.address,
  })).result.account_objects)

  console.log((await client.request({
    command: 'account_objects',
    account: customer.address,
  })).result.account_objects)

  await client.disconnect()
}

test()

Results from the JS code: (the trustline has not been deleted from either party)

➜  cmake-build-debug git:(AuthFlag) ✗ node trustLineDel.js | tee resetLimitTo0                   
connected
Creating trust line from customer's end

{
  id: 30,
  result: {
    Account: 'rBDHZdESQMrFjaJ9w1DrGyM6WbdA7s9Pp9',
    Fee: '12',
    Flags: 0,
    LastLedgerSequence: 43559904,
    LimitAmount: {
      currency: 'USD',
      issuer: 'rsUMS4sCLnNZLHdDwAcgjkUjxN3aHPU6ke',
      value: '2'
    },
    Sequence: 43559884,
    SigningPubKey: 'ED5F2E1A61A51EA56FAE827524DE20F661738C4EC3DB42875D4FA68278797D9555',
    TransactionType: 'TrustSet',
    TxnSignature: '190AB2CFBC174097D8661A33FF6C231BC03948132ED518D94C1E39644DB08F887EDF48DDFAB4D4BC2E3BE5A2DD60C31561A96CFD5FEEC00B15DF2D4CA1E73B03',
    ctid: 'C298ABCE00000001',
    date: 755389311,
    hash: '1F0C8650127C8D2C95414C2825C0773A1E08197CCDCDDFA13197B85BF1DEEB77',
    inLedger: 43559886,
    ledger_index: 43559886,
    meta: {
      AffectedNodes: [Array],
      TransactionIndex: 0,
      TransactionResult: 'tesSUCCESS'
    },
    validated: true
  },
  type: 'response'
}
Creating trust line from issuer's end

{
  id: 44,
  result: {
    Account: 'rsUMS4sCLnNZLHdDwAcgjkUjxN3aHPU6ke',
    Fee: '12',
    Flags: 0,
    LastLedgerSequence: 43559906,
    LimitAmount: {
      currency: 'USD',
      issuer: 'rBDHZdESQMrFjaJ9w1DrGyM6WbdA7s9Pp9',
      value: '0'
    },
    Sequence: 43559882,
    SigningPubKey: 'ED27292B71AA4297D2301501CE68DAB7DD0D69D6083CD7A404B22CF386E6068D9D',
    TransactionType: 'TrustSet',
    TxnSignature: 'CCECD8CB70152342970832FA1ACD787CEC562CD7F0079693E615B961B4213F14EF36C27D91D0F27E8CC194822697B7D4E9305A2C7C89CD463D9D5CCCE74AAE0B',
    ctid: 'C298ABD000000001',
    date: 755389320,
    hash: 'FEF1023598540602D2CF1F007EE648683906D5E3F2DC1A77112A15604D064AF9',
    inLedger: 43559888,
    ledger_index: 43559888,
    meta: {
      AffectedNodes: [Array],
      TransactionIndex: 0,
      TransactionResult: 'tesSUCCESS'
    },
    validated: true
  },
  type: 'response'
}
{
  id: 60,
  result: {
    Account: 'rBDHZdESQMrFjaJ9w1DrGyM6WbdA7s9Pp9',
    Fee: '12',
    Flags: 0,
    LastLedgerSequence: 43559908,
    LimitAmount: {
      currency: 'USD',
      issuer: 'rsUMS4sCLnNZLHdDwAcgjkUjxN3aHPU6ke',
      value: '0'
    },
    Sequence: 43559885,
    SigningPubKey: 'ED5F2E1A61A51EA56FAE827524DE20F661738C4EC3DB42875D4FA68278797D9555',
    TransactionType: 'TrustSet',
    TxnSignature: '27BAE05F2FDCBEC0824C8ECF6CDD74BB8DD417330578BB3835708F3723D2D54CC82E50AB9035F672516A153FC93BAF6943477CEB39E3ADF021CDA0A9CBD72D01',
    ctid: 'C298ABD200020001',
    date: 755389330,
    hash: '7ADC9F28F08B91625396EF1AC13C54307F113786A8B2514FEB5C1E7055EB83D4',
    inLedger: 43559890,
    ledger_index: 43559890,
    meta: {
      AffectedNodes: [Array],
      TransactionIndex: 2,
      TransactionResult: 'tesSUCCESS'
    },
    validated: true
  },
  type: 'response'
}
Completed the reset of trustline from customer's perspective. Account objects for issuer and customer resp

[
  {
    Balance: {
      currency: 'USD',
      issuer: 'rrrrrrrrrrrrrrrrrrrrBZbvji',
      value: '0'
    },
    Flags: 1179648,
    HighLimit: {
      currency: 'USD',
      issuer: 'rBDHZdESQMrFjaJ9w1DrGyM6WbdA7s9Pp9',
      value: '0'
    },
    HighNode: '0',
    LedgerEntryType: 'RippleState',
    LowLimit: {
      currency: 'USD',
      issuer: 'rsUMS4sCLnNZLHdDwAcgjkUjxN3aHPU6ke',
      value: '0'
    },
    LowNode: '0',
    PreviousTxnID: '7ADC9F28F08B91625396EF1AC13C54307F113786A8B2514FEB5C1E7055EB83D4',
    PreviousTxnLgrSeq: 43559890,
    index: '0BD2358A2AF0A23AB5743B9C9D7FC4B5F6E4C1CB65D39467D67F20E7A9763417'
  }
]
[
  {
    Balance: {
      currency: 'USD',
      issuer: 'rrrrrrrrrrrrrrrrrrrrBZbvji',
      value: '0'
    },
    Flags: 1179648,
    HighLimit: {
      currency: 'USD',
      issuer: 'rBDHZdESQMrFjaJ9w1DrGyM6WbdA7s9Pp9',
      value: '0'
    },
    HighNode: '0',
    LedgerEntryType: 'RippleState',
    LowLimit: {
      currency: 'USD',
      issuer: 'rsUMS4sCLnNZLHdDwAcgjkUjxN3aHPU6ke',
      value: '0'
    },
    LowNode: '0',
    PreviousTxnID: '7ADC9F28F08B91625396EF1AC13C54307F113786A8B2514FEB5C1E7055EB83D4',
    PreviousTxnLgrSeq: 43559890,
    index: '0BD2358A2AF0A23AB5743B9C9D7FC4B5F6E4C1CB65D39467D67F20E7A9763417'
  }
]

I have modified your code slightly to demonstrate this issue. I'm not sure why the js and C++ code differ in their behavior. Am I missing something?

@mvadari
Copy link
Collaborator

mvadari commented Dec 8, 2023

It looks like you're never actually authorizing the trustline. The issuer needs to authorize it, not the customer. In this case, auth1 is the issuer and auth2 is the customer.

Some things that make that testcase confusing to understand:

  • Both accounts have asfRequireAuth enabled
  • The trustline is created with USD2 but checked with USD1
  • You don't check if there's still a trustline owned by auth2

@ckeshava
Copy link
Collaborator

Okay, I've fixed the mistakes in the previous iteration. Here's the unit test : develop...ckeshava:rippled:AuthFlag

If you reset the limits of the trust lines to the default, they are deleted from the accounts. This holds true for Authorised Trust lines too.

This is incoherent with a section of the documentation
image

@ckeshava
Copy link
Collaborator

@mvadari I have an update on this issue.

There is an explanation for the difference in the behavior of C++ Unit tests and the Typescript libraries. many thanks to @ckniffen for brain-storming with me and helping me debug this issue.

When new accounts are created in C++ unit tests, "defaultRipple" : true is set in the AccountRoot object. On the other hand, the Client class in Typescript sets defaultRipple: false.

The behavior of C++ unit tests is documented here:

fund(bool setDefaultRipple, STAmount const& amount, Account const& account);
I don't know if developers have a similar understanding of the TypeScript library code.

Typescript's generateFundedWallet sets the defaultRipple to false somewhere in its internals, I don't know the exact location yet.

Due to the above reasons, trust lines created in the two languages have different properties.

{ lsfHighReserve: true } // Trust lines created from C++ unit tests
{ lsfHighReserve: true, lsfLowNoRipple: true } // Trust lines created from Typescript Client class

Since the trust lines generated from Typescript is not in the default state, they are not deleted even if developers update balance and limit trust line settings to 0. Note: This behavior is observed even without the use of lsfAuthFlag. I didn't use that flag in my experiments to keep it simple.

I haven't debugged the python library yet, I'm facing a tangential issue on that front.

If this is a well-known intended behavior, then I'd recommend highlighting it in the docs. Otherwise, I'll look for specific places in the Typescript code that's causing this discrepency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Amendment Bug Good First Issue Great issue for a new contributor
Projects
None yet
Development

No branches or pull requests

4 participants