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

Jump to bottom

Add AMMClawback Transaction (XLS-0073d) #5142

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open

Add AMMClawback Transaction (XLS-0073d) #5142

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9b30183
Add AMMClawback Transaction
yinyiqian1 Sep 23, 2024
3be6878
Use asset and asset2 to navigate AMM
yinyiqian1 Sep 24, 2024
01cbe02
Use tfee as 0 since it is a two asset withdrawal
yinyiqian1 Sep 24, 2024
4bdb019
resolve review comments
yinyiqian1 Sep 27, 2024
2796e8d
use fhIGNORE_FREEZE for AMMClawback
yinyiqian1 Oct 1, 2024
e449a09
change withdraw ammSle parameter order
yinyiqian1 Oct 1, 2024
9f714bb
resolve error type and other comments
yinyiqian1 Oct 2, 2024
860d033
Change temBAD_AMOUNT error message
yinyiqian1 Oct 3, 2024
d3d7292
Merge branch 'develop' into amm-clawback
yinyiqian1 Oct 3, 2024
a15db14
Block unauthorized account to deposit into AMM pool
yinyiqian1 Oct 14, 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
3 changes: 2 additions & 1 deletion include/xrpl/protocol/Feature.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ namespace detail {
// Feature.cpp. Because it's only used to reserve storage, and determine how
// large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
// the actual number of amendments. A LogicError on startup will verify this.
static constexpr std::size_t numFeatures = 79;
static constexpr std::size_t numFeatures = 80;

/** Amendments that this server supports and the default voting behavior.
Whether they are enabled depends on the Rules defined in the validated
Expand Down Expand Up @@ -372,6 +372,7 @@ extern uint256 const fixEnforceNFTokenTrustline;
extern uint256 const fixInnerObjTemplate2;
extern uint256 const featureInvariantsV1_1;
extern uint256 const fixNFTokenPageLinks;
extern uint256 const featureAMMClawback;

} // namespace ripple

Expand Down
1 change: 1 addition & 0 deletions include/xrpl/protocol/SField.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,7 @@ extern SF_ACCOUNT const sfUnauthorize;
extern SF_ACCOUNT const sfRegularKey;
extern SF_ACCOUNT const sfNFTokenMinter;
extern SF_ACCOUNT const sfEmitCallback;
extern SF_ACCOUNT const sfHolder;

// account (uncommon)
extern SF_ACCOUNT const sfHookAccount;
Expand Down
2 changes: 2 additions & 0 deletions include/xrpl/protocol/TER.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ enum TEMcodes : TERUnderlyingType {

temARRAY_EMPTY,
temARRAY_TOO_LARGE,
temBAD_ASSET_AMOUNT,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use temBAD_AMOUN and temBAD_ISSUER instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There's a temBAD_AMOUNT already defined for non-positive amount errors.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's what I mean, don't need to define new ones.

Copy link
Collaborator Author

@yinyiqian1 yinyiqian1 Sep 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Then the request's error message for this one will be inaccurate: Can only send positive amounts.
But I intended to return error message as Malformed: Amount does not match Asset.
temBAD_AMOUNT is only for positive amount check.
I think defining this new error will benefit if in the future, we need to check if asset matches with amount again.

Copy link
Collaborator

Choose a reason for hiding this comment

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

could just use temMALFORMED then.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's also thrown in the escrow code if the asset isn't XRP.

Copy link
Collaborator Author

@yinyiqian1 yinyiqian1 Oct 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

in escrow code, if it's using temBAD_AMOUNT, the error message will be "Can only send positive amounts.". It does not match with that the asset isn't XRP. I think in escrow, it should just use temMalformed or define more accurate error type. Maybe we should enrich our error code to return more accurate error messages. Because these errors can be used in the future.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not just change the error message, so that it makes sense both in Escrow and here, instead of needing a new error code? I believe changing the error message isn't considered a breaking change/needing an amendment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can change the message for temBAD_AMOUNT to "bad amount provided". What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Malformed: Bad amount. would keep it in line with the others.

temBAD_ASSET_ISSUER
};

//------------------------------------------------------------------------------
Expand Down
4 changes: 4 additions & 0 deletions include/xrpl/protocol/TxFlags.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@ constexpr std::uint32_t tfDepositSubTx =
constexpr std::uint32_t tfWithdrawMask = ~(tfUniversal | tfWithdrawSubTx);
constexpr std::uint32_t tfDepositMask = ~(tfUniversal | tfDepositSubTx);

// AMMClawback flags:
constexpr std::uint32_t tfClawTwoAssets = 0x00000001;
constexpr std::uint32_t tfAMMClawbackMask = ~(tfUniversal | tfClawTwoAssets);

// BridgeModify flags:
constexpr std::uint32_t tfClearAccountCreateAmount = 0x00010000;
constexpr std::uint32_t tfBridgeModifyMask = ~(tfUniversal | tfClearAccountCreateAmount);
Expand Down
3 changes: 3 additions & 0 deletions include/xrpl/protocol/TxFormats.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ enum TxType : std::uint16_t
/** This transaction claws back issued tokens. */
ttCLAWBACK = 30,

/** This transaction claws back tokens from an AMM pool. */
ttAMM_CLAWBACK = 31,

/** This transaction type creates an AMM instance */
ttAMM_CREATE = 35,

Expand Down
2 changes: 2 additions & 0 deletions include/xrpl/protocol/jss.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ JSS(AccountRoot); // ledger type.
JSS(AccountSet); // transaction type.
JSS(AMM); // ledger type
JSS(AMMBid); // transaction type
JSS(AMMClawback); // transaction type.
JSS(AMMID); // field
JSS(AMMCreate); // transaction type
JSS(AMMDeposit); // transaction type
Expand Down Expand Up @@ -89,6 +90,7 @@ JSS(EscrowFinish); // transaction type.
JSS(Fee); // in/out: TransactionSign; field.
JSS(FeeSettings); // ledger type.
JSS(Flags); // in/out: TransactionSign; field.
JSS(Holder); // field.
JSS(Invalid); //
JSS(LastLedgerSequence); // in: TransactionSign; field
JSS(LastUpdateTime); // field.
Expand Down
1 change: 1 addition & 0 deletions src/libxrpl/protocol/Feature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,7 @@ REGISTER_FIX (fixNFTokenPageLinks, Supported::yes, VoteBehavior::De
// InvariantsV1_1 will be changes to Supported::yes when all the
// invariants expected to be included under it are complete.
REGISTER_FEATURE(InvariantsV1_1, Supported::no, VoteBehavior::DefaultNo);
REGISTER_FEATURE(AMMClawback, Supported::yes, VoteBehavior::DefaultNo);

// The following amendments are obsolete, but must remain supported
// because they could potentially get enabled.
Expand Down
1 change: 1 addition & 0 deletions src/libxrpl/protocol/SField.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ CONSTRUCT_TYPED_SFIELD(sfUnauthorize, "Unauthorize", ACCOUNT,
CONSTRUCT_TYPED_SFIELD(sfRegularKey, "RegularKey", ACCOUNT, 8);
CONSTRUCT_TYPED_SFIELD(sfNFTokenMinter, "NFTokenMinter", ACCOUNT, 9);
CONSTRUCT_TYPED_SFIELD(sfEmitCallback, "EmitCallback", ACCOUNT, 10);
CONSTRUCT_TYPED_SFIELD(sfHolder, "Holder", ACCOUNT, 11);

// account (uncommon)
CONSTRUCT_TYPED_SFIELD(sfHookAccount, "HookAccount", ACCOUNT, 16);
Expand Down
2 changes: 2 additions & 0 deletions src/libxrpl/protocol/TER.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,8 @@ transResults()
MAKE_ERROR(temXCHAIN_BRIDGE_BAD_REWARD_AMOUNT, "Malformed: Bad reward amount."),
MAKE_ERROR(temARRAY_EMPTY, "Malformed: Array is empty."),
MAKE_ERROR(temARRAY_TOO_LARGE, "Malformed: Array is too large."),
MAKE_ERROR(temBAD_ASSET_AMOUNT, "Malformed: Amount does not match Asset."),
MAKE_ERROR(temBAD_ASSET_ISSUER, "Malformed: Issuer does not match Asset."),
gregtatcam marked this conversation as resolved.
Show resolved Hide resolved

MAKE_ERROR(terRETRY, "Retry transaction."),
MAKE_ERROR(terFUNDS_SPENT, "DEPRECATED."),
Expand Down
10 changes: 10 additions & 0 deletions src/libxrpl/protocol/TxFormats.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,16 @@ TxFormats::TxFormats()
},
commonFields);

add(jss::AMMClawback,
ttAMM_CLAWBACK,
{
{sfHolder, soeREQUIRED},
{sfAsset, soeREQUIRED},
{sfAsset2, soeREQUIRED},
{sfAmount, soeOPTIONAL},
},
commonFields);

add(jss::XChainCreateBridge,
ttXCHAIN_CREATE_BRIDGE,
{
Expand Down
Loading
Loading