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.

Sign up for GitHub

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

fix maximum amt #34

Merged
merged 3 commits into from
Sep 10, 2024
Merged

fix maximum amt #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fc74112
fix maximum amt
shawnxie999 Sep 6, 2024
2a3e887
Merge remote-tracking branch 'greg/feature/mpt-v1' into fix-exceed-ma…
shawnxie999 Sep 9, 2024
3657fb2
add if check to return tecMPT_ISSUANCE_NOT_FOUND
shawnxie999 Sep 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/test/app/MPToken_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1015,6 +1015,28 @@ class MPToken_test : public beast::unit_test::suite
// destroyed, it should fail
mptAlice.pay(alice, bob, 100, tecMPT_ISSUANCE_NOT_FOUND);
}

// Issuers issues maximum amount of MPT to a holder, the holder should
// be able to transfer the max amount to someone else
{
Env env{*this, features};
Account const alice("alice");
Account const carol("bob");
Account const bob("carol");

MPTTester mptAlice(env, alice, {.holders = {&bob, &carol}});

mptAlice.create(
{.maxAmt = 100, .ownerCount = 1, .flags = tfMPTCanTransfer});

mptAlice.authorize({.account = &bob});
mptAlice.authorize({.account = &carol});

mptAlice.pay(alice, bob, 100);

// transfer max amount to another holder
mptAlice.pay(bob, carol, 100);
}
}

void
Expand Down
22 changes: 16 additions & 6 deletions src/xrpld/ledger/detail/View.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1353,6 +1353,20 @@ rippleSend(

if (uSenderID == issuer || uReceiverID == issuer || issuer == noAccount())
{
// if sender is issuer, check that the new OutstandingAmount will not
// exceed MaximumAmount
if (uSenderID == issuer)
{
auto const mptID = keylet::mptIssuance(saAmount.issue().getMptID());
auto const sle = view.peek(mptID);
if (!sle)
return tecMPT_ISSUANCE_NOT_FOUND;

if (sle->getFieldU64(sfOutstandingAmount) + saAmount.value() >
(*sle)[~sfMaximumAmount].value_or(maxMPTokenAmount))
return tecMPT_MAX_AMOUNT_EXCEEDED;
Copy link
Owner

Choose a reason for hiding this comment

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

Should add a unit-test for this.

Copy link
Author

Choose a reason for hiding this comment

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

we already have existing unit tests that tests if the issuer issues out more than the max. the unit test i added covered the scenario reproduced in QA that currently exhibits the wrong behavior

Copy link
Owner

Choose a reason for hiding this comment

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

Isn't this a different execution path?

Copy link
Author

Choose a reason for hiding this comment

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

All the existing test cases hit this new if-check in rippleSend. The problem found is that during holder to holder payment, rippleCredit is called twice from 1) holder to issuer and then 2)issuer to holder. The current problem is that in the second rippleCredit call from issuer to holder, the code think the issuer is trying to issue more than the MaximumAmount. This is why I moved the check in the rippleCredit to its caller rippleSend.

}

// Direct send: redeeming IOUs and/or sending own IOUs.
auto const ter =
rippleCredit(view, uSenderID, uReceiverID, saAmount, j);
Expand Down Expand Up @@ -1380,8 +1394,8 @@ rippleSend(
rippleCredit(view, issuer, uReceiverID, saAmount, j);
terResult != tesSUCCESS)
return terResult;
else
return rippleCredit(view, uSenderID, issuer, saActual, j);

return rippleCredit(view, uSenderID, issuer, saActual, j);
}

return tecINTERNAL;
Expand Down Expand Up @@ -1872,10 +1886,6 @@ rippleCredit(
sfOutstandingAmount,
sle->getFieldU64(sfOutstandingAmount) + saAmount.value());

if (sle->getFieldU64(sfOutstandingAmount) >
(*sle)[~sfMaximumAmount].value_or(maxMPTokenAmount))
return tecMPT_MAX_AMOUNT_EXCEEDED;

view.update(sle);
}
else
Expand Down
Loading