From 59e334c099c9bd4e4c4d06347dd7095e68490140 Mon Sep 17 00:00:00 2001 From: tequ Date: Tue, 15 Apr 2025 19:08:19 +0900 Subject: [PATCH 1/5] fixRewardClaimFlags (#487) --- src/ripple/app/tx/impl/ClaimReward.cpp | 7 +++++-- src/ripple/protocol/Feature.h | 3 ++- src/ripple/protocol/TxFlags.h | 1 + src/ripple/protocol/impl/Feature.cpp | 1 + src/test/app/ClaimReward_test.cpp | 21 +++++++++++++++++++++ 5 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/ripple/app/tx/impl/ClaimReward.cpp b/src/ripple/app/tx/impl/ClaimReward.cpp index 37b065083..b87777dc0 100644 --- a/src/ripple/app/tx/impl/ClaimReward.cpp +++ b/src/ripple/app/tx/impl/ClaimReward.cpp @@ -45,8 +45,11 @@ ClaimReward::preflight(PreflightContext const& ctx) return ret; // can have flag 1 set to opt-out of rewards - if (ctx.tx.isFieldPresent(sfFlags) && - ctx.tx.getFieldU32(sfFlags) > tfOptOut) + auto const invalidFlags = ctx.rules.enabled(fixRewardClaimFlags) + ? (ctx.tx.getFlags() & tfClaimRewardMask) + : (ctx.tx.isFieldPresent(sfFlags) && + ctx.tx.getFieldU32(sfFlags) > tfOptOut); + if (invalidFlags) return temINVALID_FLAG; if (ctx.tx.isFieldPresent(sfIssuer) && diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index e06bb7c11..7661b962c 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -74,7 +74,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 @@ -367,6 +367,7 @@ extern uint256 const fixReduceImport; extern uint256 const fixXahauV3; extern uint256 const fix20250131; extern uint256 const featureHookCanEmit; +extern uint256 const fixRewardClaimFlags; } // namespace ripple diff --git a/src/ripple/protocol/TxFlags.h b/src/ripple/protocol/TxFlags.h index b12558200..24b58b596 100644 --- a/src/ripple/protocol/TxFlags.h +++ b/src/ripple/protocol/TxFlags.h @@ -189,6 +189,7 @@ constexpr std::uint32_t const tfURITokenNonMintMask = ~tfUniversal; enum ClaimRewardFlags : uint32_t { tfOptOut = 0x00000001, }; +constexpr std::uint32_t const tfClaimRewardMask = ~(tfUniversal | tfOptOut); // clang-format on diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index f285574ff..35ada0af7 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -473,6 +473,7 @@ REGISTER_FIX (fixReduceImport, Supported::yes, VoteBehavior::De REGISTER_FIX (fixXahauV3, Supported::yes, VoteBehavior::DefaultYes); REGISTER_FIX (fix20250131, Supported::yes, VoteBehavior::DefaultYes); REGISTER_FEATURE(HookCanEmit, Supported::yes, VoteBehavior::DefaultNo); +REGISTER_FIX (fixRewardClaimFlags, Supported::yes, VoteBehavior::DefaultYes); // The following amendments are obsolete, but must remain supported // because they could potentially get enabled. diff --git a/src/test/app/ClaimReward_test.cpp b/src/test/app/ClaimReward_test.cpp index 85a6ba62e..b2c937ef7 100644 --- a/src/test/app/ClaimReward_test.cpp +++ b/src/test/app/ClaimReward_test.cpp @@ -186,6 +186,27 @@ struct ClaimReward_test : public beast::unit_test::suite ter(temINVALID_FLAG)); env.close(); } + { + for (bool const withFixFlags : {false, true}) + { + auto const amend = + withFixFlags ? features : features - fixRewardClaimFlags; + Env env{*this, amend}; + + auto const alice = Account("alice"); + auto const issuer = Account("issuer"); + + env.fund(XRP(1000), alice, issuer); + env.close(); + + auto tx = reward::claim(alice); + env(tx, + reward::issuer(issuer), + txflags(tfFullyCanonicalSig), + withFixFlags ? ter(tesSUCCESS) : ter(temINVALID_FLAG)); + env.close(); + } + } // temMALFORMED // Issuer cannot be the source account. From f9cd2e0d21e623b5be5526df86eb4791c85b3a15 Mon Sep 17 00:00:00 2001 From: RichardAH Date: Wed, 16 Apr 2025 08:37:58 +1000 Subject: [PATCH 2/5] Remarks amendment (#301) Co-authored-by: Denis Angell --- Builds/CMake/RippledCore.cmake | 7 +- src/ripple/app/tx/impl/Remit.h | 4 +- src/ripple/app/tx/impl/SetRemarks.cpp | 450 +++++++++++++ src/ripple/app/tx/impl/SetRemarks.h | 60 ++ src/ripple/app/tx/impl/applySteps.cpp | 11 + src/ripple/protocol/Feature.h | 3 +- src/ripple/protocol/LedgerFormats.h | 3 + src/ripple/protocol/SField.h | 5 + src/ripple/protocol/TER.h | 2 + src/ripple/protocol/TxFlags.h | 3 + src/ripple/protocol/TxFormats.h | 3 + src/ripple/protocol/impl/Feature.cpp | 1 + .../protocol/impl/InnerObjectFormats.cpp | 8 + src/ripple/protocol/impl/LedgerFormats.cpp | 1 + src/ripple/protocol/impl/SField.cpp | 5 + src/ripple/protocol/impl/TER.cpp | 2 + src/ripple/protocol/impl/TxFormats.cpp | 8 + src/ripple/protocol/jss.h | 1 + src/test/app/SetRemarks_test.cpp | 591 ++++++++++++++++++ src/test/jtx.h | 1 + src/test/jtx/impl/remarks.cpp | 56 ++ src/test/jtx/remarks.h | 64 ++ 22 files changed, 1284 insertions(+), 5 deletions(-) create mode 100644 src/ripple/app/tx/impl/SetRemarks.cpp create mode 100644 src/ripple/app/tx/impl/SetRemarks.h create mode 100644 src/test/app/SetRemarks_test.cpp create mode 100644 src/test/jtx/impl/remarks.cpp create mode 100644 src/test/jtx/remarks.h diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index 8f44b97af..bbafd892e 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -456,6 +456,7 @@ target_sources (rippled PRIVATE src/ripple/app/tx/impl/Remit.cpp src/ripple/app/tx/impl/SetAccount.cpp src/ripple/app/tx/impl/SetHook.cpp + src/ripple/app/tx/impl/SetRemarks.cpp src/ripple/app/tx/impl/SetRegularKey.cpp src/ripple/app/tx/impl/SetSignerList.cpp src/ripple/app/tx/impl/SetTrust.cpp @@ -752,7 +753,10 @@ if (tests) src/test/app/Remit_test.cpp src/test/app/SHAMapStore_test.cpp src/test/app/SetAuth_test.cpp + src/test/app/SetHook_test.cpp + src/test/app/SetHookTSH_test.cpp src/test/app/SetRegularKey_test.cpp + src/test/app/SetRemarks_test.cpp src/test/app/SetTrust_test.cpp src/test/app/Taker_test.cpp src/test/app/TheoreticalQuality_test.cpp @@ -765,8 +769,6 @@ if (tests) src/test/app/ValidatorKeys_test.cpp src/test/app/ValidatorList_test.cpp src/test/app/ValidatorSite_test.cpp - src/test/app/SetHook_test.cpp - src/test/app/SetHookTSH_test.cpp src/test/app/Wildcard_test.cpp src/test/app/XahauGenesis_test.cpp src/test/app/tx/apply_test.cpp @@ -900,6 +902,7 @@ if (tests) src/test/jtx/impl/rate.cpp src/test/jtx/impl/regkey.cpp src/test/jtx/impl/reward.cpp + src/test/jtx/impl/remarks.cpp src/test/jtx/impl/remit.cpp src/test/jtx/impl/sendmax.cpp src/test/jtx/impl/seq.cpp diff --git a/src/ripple/app/tx/impl/Remit.h b/src/ripple/app/tx/impl/Remit.h index 56ac494be..e0003daf7 100644 --- a/src/ripple/app/tx/impl/Remit.h +++ b/src/ripple/app/tx/impl/Remit.h @@ -17,8 +17,8 @@ */ //============================================================================== -#ifndef RIPPLE_TX_SIMPLE_PAYMENT_H_INCLUDED -#define RIPPLE_TX_SIMPLE_PAYMENT_H_INCLUDED +#ifndef RIPPLE_TX_REMIT_H_INCLUDED +#define RIPPLE_TX_REMIT_H_INCLUDED #include #include diff --git a/src/ripple/app/tx/impl/SetRemarks.cpp b/src/ripple/app/tx/impl/SetRemarks.cpp new file mode 100644 index 000000000..f45479bf5 --- /dev/null +++ b/src/ripple/app/tx/impl/SetRemarks.cpp @@ -0,0 +1,450 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 XRPL-Labs + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace ripple { + +TxConsequences +SetRemarks::makeTxConsequences(PreflightContext const& ctx) +{ + return TxConsequences{ctx.tx, TxConsequences::normal}; +} + +NotTEC +SetRemarks::validateRemarks(STArray const& remarks, beast::Journal const& j) +{ + std::set already_seen; + + if (remarks.empty() || remarks.size() > 32) + { + JLOG(j.warn()) << "SetRemarks: Cannot set more than 32 remarks (or " + "fewer than 1) in a txn."; + return temMALFORMED; + } + + for (auto const& remark : remarks) + { + if (remark.getFName() != sfRemark) + { + JLOG(j.warn()) << "SetRemarks: contained non-sfRemark field."; + return temMALFORMED; + } + + // will be checked by template system, extra check for security + if (!remark.isFieldPresent(sfRemarkName)) + return temMALFORMED; + + Blob const& name = remark.getFieldVL(sfRemarkName); + if (name.size() == 0 || name.size() > 256) + { + JLOG(j.warn()) << "SetRemarks: RemarkName cannot be empty or " + "larger than 256 chars."; + return temMALFORMED; + } + + if (already_seen.find(name) != already_seen.end()) + { + JLOG(j.warn()) << "SetRemarks: duplicate RemarkName entry."; + return temMALFORMED; + } + + already_seen.emplace(name); + + uint32_t flags = + remark.isFieldPresent(sfFlags) ? remark.getFieldU32(sfFlags) : 0; + if (flags != 0 && flags != tfImmutable) + { + JLOG(j.warn()) + << "SetRemarks: Flags must be either tfImmutable or 0"; + return temMALFORMED; + } + + if (!remark.isFieldPresent(sfRemarkValue)) + { + if (flags & tfImmutable) + { + JLOG(j.warn()) << "SetRemarks: A remark deletion cannot be " + "marked immutable."; + return temMALFORMED; + } + continue; + } + + Blob const& val = remark.getFieldVL(sfRemarkValue); + if (val.size() == 0 || val.size() > 256) + { + JLOG(j.warn()) << "SetRemarks: RemarkValue cannot be empty or " + "larger than 256 chars."; + return temMALFORMED; + } + } + + return tesSUCCESS; +} + +NotTEC +SetRemarks::preflight(PreflightContext const& ctx) +{ + if (!ctx.rules.enabled(featureRemarks)) + return temDISABLED; + + if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + return ret; + + auto& tx = ctx.tx; + auto& j = ctx.j; + + if (tx.getFlags() & tfUniversalMask) + { + JLOG(j.warn()) << "SetRemarks: Invalid flags set."; + return temINVALID_FLAG; + } + + auto const& remarks = tx.getFieldArray(sfRemarks); + if (NotTEC result = validateRemarks(remarks, j); !isTesSuccess(result)) + return result; + + return preflight2(ctx); +} + +template +inline std::optional +getRemarksIssuer(T const& sleO) +{ + std::optional issuer; + + // check if it's an allowable object type + uint16_t lt = sleO->getFieldU16(sfLedgerEntryType); + + switch (lt) + { + case ltACCOUNT_ROOT: + case ltOFFER: + case ltESCROW: + case ltTICKET: + case ltPAYCHAN: + case ltCHECK: + case ltDEPOSIT_PREAUTH: { + issuer = sleO->getAccountID(sfAccount); + break; + } + + case ltNFTOKEN_OFFER: { + issuer = sleO->getAccountID(sfOwner); + break; + } + + case ltURI_TOKEN: { + issuer = sleO->getAccountID(sfIssuer); + break; + } + + case ltRIPPLE_STATE: { + // remarks can only be attached to a trustline by the issuer + AccountID lowAcc = sleO->getFieldAmount(sfLowLimit).getIssuer(); + AccountID highAcc = sleO->getFieldAmount(sfHighLimit).getIssuer(); + + STAmount bal = sleO->getFieldAmount(sfBalance); + + if (bal < beast::zero) + { + // low account is issuer + issuer = lowAcc; + break; + } + + if (bal > beast::zero) + { + // high acccount is issuer + issuer = highAcc; + break; + } + + // if the balance is zero we'll look for the side in default state + // and assume this is the issuer + uint32_t flags = sleO->getFieldU32(sfFlags); + bool const highReserve = (flags & lsfHighReserve); + bool const lowReserve = (flags & lsfLowReserve); + + if (!highReserve && !lowReserve) + { + // error state + // do nothing, fallthru. + } + else if (highReserve && lowReserve) + { + // in this edge case we don't know who is the issuer, because + // there isn't a clear issuer. do nothing, fallthru. + } + else + { + issuer = (highReserve ? lowAcc : highAcc); + break; + } + } + } + + return issuer; +} + +TER +SetRemarks::preclaim(PreclaimContext const& ctx) +{ + if (!ctx.view.rules().enabled(featureRemarks)) + return temDISABLED; + + auto const id = ctx.tx[sfAccount]; + + auto const sle = ctx.view.read(keylet::account(id)); + if (!sle) + return terNO_ACCOUNT; + + auto const objID = ctx.tx[sfObjectID]; + auto const sleO = ctx.view.read(keylet::unchecked(objID)); + if (!sleO) + return tecNO_TARGET; + + std::optional issuer = getRemarksIssuer(sleO); + + if (!issuer || *issuer != id) + return tecNO_PERMISSION; + + // sanity check the remarks merge between txn and obj + auto const& remarksTxn = ctx.tx.getFieldArray(sfRemarks); + + std::map> keys; + if (sleO->isFieldPresent(sfRemarks)) + { + auto const& remarksObj = sleO->getFieldArray(sfRemarks); + + // map the remark name to its value and whether it's immutable + for (auto const& remark : remarksObj) + keys.emplace(std::make_pair( + remark.getFieldVL(sfRemarkName), + std::make_pair( + remark.getFieldVL(sfRemarkValue), + remark.isFieldPresent(sfFlags) && + remark.getFieldU32(sfFlags) & tfImmutable))); + } + + int64_t count = keys.size(); + + for (auto const& remark : remarksTxn) + { + std::optional valTxn; + if (remark.isFieldPresent(sfRemarkValue)) + valTxn = remark.getFieldVL(sfRemarkValue); + bool const isDeletion = !valTxn.has_value(); + + Blob name = remark.getFieldVL(sfRemarkName); + if (keys.find(name) == keys.end()) + { + // new remark + if (isDeletion) + { + // this could have been an error but deleting something + // that doesn't exist is traditionally not an error in xrpl + continue; + } + + ++count; + continue; + } + + auto const& [valObj, immutable] = keys[name]; + + // even if it's mutable, if we don't mutate it that's a noop so just + // pass it + if (valTxn.has_value() && *valTxn == valObj) + continue; + + if (immutable) + { + JLOG(ctx.j.warn()) + << "SetRemarks: attempt to mutate an immutable remark."; + return tecIMMUTABLE; + } + + if (isDeletion) + { + if (--count < 0) + { + JLOG(ctx.j.warn()) << "SetRemarks: insane remarks accounting."; + return tecCLAIM; + } + } + } + + if (count > 32) + { + JLOG(ctx.j.warn()) + << "SetRemarks: an object may have at most 32 remarks."; + return tecTOO_MANY_REMARKS; + } + + return tesSUCCESS; +} + +TER +SetRemarks::doApply() +{ + auto j = ctx_.journal; + Sandbox sb(&ctx_.view()); + + auto const sle = sb.read(keylet::account(account_)); + if (!sle) + return terNO_ACCOUNT; + + auto const objID = ctx_.tx[sfObjectID]; + auto sleO = sb.peek(keylet::unchecked(objID)); + if (!sleO) + return terNO_ACCOUNT; + + std::optional issuer = getRemarksIssuer(sleO); + + if (!issuer || *issuer != account_) + return tecNO_PERMISSION; + + auto const& remarksTxn = ctx_.tx.getFieldArray(sfRemarks); + + std::map> remarksMap; + + if (sleO->isFieldPresent(sfRemarks)) + { + auto const& remarksObj = sleO->getFieldArray(sfRemarks); + for (auto const& remark : remarksObj) + { + uint32_t flags = remark.isFieldPresent(sfFlags) + ? remark.getFieldU32(sfFlags) + : 0; + bool const immutable = (flags & tfImmutable) != 0; + remarksMap[remark.getFieldVL(sfRemarkName)] = { + remark.getFieldVL(sfRemarkValue), + remark.isFieldPresent(sfFlags) && immutable}; + } + } + + for (auto const& remark : remarksTxn) + { + std::optional val; + if (remark.isFieldPresent(sfRemarkValue)) + val = remark.getFieldVL(sfRemarkValue); + Blob name = remark.getFieldVL(sfRemarkName); + + bool const isDeletion = !val.has_value(); + uint32_t flags = + remark.isFieldPresent(sfFlags) ? remark.getFieldU32(sfFlags) : 0; + bool const setImmutable = (flags & tfImmutable) != 0; + + if (isDeletion) + { + if (remarksMap.find(name) != remarksMap.end()) + remarksMap.erase(name); + continue; + } + + if (remarksMap.find(name) == remarksMap.end()) + { + remarksMap[name] = std::make_pair(*val, setImmutable); + continue; + } + + remarksMap[name].first = *val; + if (!remarksMap[name].second) + remarksMap[name].second = setImmutable; + } + + // canonically order + std::vector keys; + for (auto const& [k, _] : remarksMap) + keys.push_back(k); + + std::sort(keys.begin(), keys.end()); + + STArray newRemarks{sfRemarks, static_cast(keys.size())}; + for (auto const& k : keys) + { + STObject remark{sfRemark}; + + remark.setFieldVL(sfRemarkName, k); + remark.setFieldVL(sfRemarkValue, remarksMap[k].first); + if (remarksMap[k].second) + remark.setFieldU32(sfFlags, lsfImmutable); + + newRemarks.push_back(std::move(remark)); + } + + if (newRemarks.size() > 32) + return tecTOO_MANY_REMARKS; + + if (newRemarks.empty() && sleO->isFieldPresent(sfRemarks)) + sleO->makeFieldAbsent(sfRemarks); + else + sleO->setFieldArray(sfRemarks, std::move(newRemarks)); + + sb.update(sleO); + sb.apply(ctx_.rawView()); + + return tesSUCCESS; +} + +XRPAmount +SetRemarks::calculateBaseFee(ReadView const& view, STTx const& tx) +{ + XRPAmount remarkFee{0}; + if (tx.isFieldPresent(sfRemarks)) + { + int64_t remarkBytes = 0; + auto const& remarks = tx.getFieldArray(sfRemarks); + for (auto const& remark : remarks) + { + int64_t entryBytes = 0; + if (remark.isFieldPresent(sfRemarkName)) + { + entryBytes += remark.getFieldVL(sfRemarkName).size(); + } + if (remark.isFieldPresent(sfRemarkValue)) + { + entryBytes += remark.getFieldVL(sfRemarkValue).size(); + } + + // overflow + if (remarkBytes + entryBytes < remarkBytes) + return INITIAL_XRP; + + remarkBytes += entryBytes; + } + + // one drop per byte + remarkFee = XRPAmount{remarkBytes}; + } + auto fee = Transactor::calculateBaseFee(view, tx); + return fee + remarkFee; +} + +} // namespace ripple diff --git a/src/ripple/app/tx/impl/SetRemarks.h b/src/ripple/app/tx/impl/SetRemarks.h new file mode 100644 index 000000000..00f74ab5d --- /dev/null +++ b/src/ripple/app/tx/impl/SetRemarks.h @@ -0,0 +1,60 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 XRPL-Labs + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#ifndef RIPPLE_TX_SETREMARKS_H_INCLUDED +#define RIPPLE_TX_SETREMARKS_H_INCLUDED + +#include +#include +#include +#include + +namespace ripple { + +class SetRemarks : public Transactor +{ +public: + static constexpr ConsequencesFactoryType ConsequencesFactory{Custom}; + + explicit SetRemarks(ApplyContext& ctx) : Transactor(ctx) + { + } + + static XRPAmount + calculateBaseFee(ReadView const& view, STTx const& tx); + + static TxConsequences + makeTxConsequences(PreflightContext const& ctx); + + static NotTEC + preflight(PreflightContext const& ctx); + + static TER + preclaim(PreclaimContext const&); + + TER + doApply() override; + + static NotTEC + validateRemarks(STArray const& remarks, beast::Journal const& j); +}; + +} // namespace ripple + +#endif diff --git a/src/ripple/app/tx/impl/applySteps.cpp b/src/ripple/app/tx/impl/applySteps.cpp index ab2bf30ca..4f29f4501 100644 --- a/src/ripple/app/tx/impl/applySteps.cpp +++ b/src/ripple/app/tx/impl/applySteps.cpp @@ -44,6 +44,7 @@ #include #include #include +#include #include #include #include @@ -169,6 +170,8 @@ invoke_preflight(PreflightContext const& ctx) return invoke_preflight_helper(ctx); case ttREMIT: return invoke_preflight_helper(ctx); + case ttREMARKS_SET: + return invoke_preflight_helper(ctx); case ttURITOKEN_MINT: case ttURITOKEN_BURN: case ttURITOKEN_BUY: @@ -290,6 +293,8 @@ invoke_preclaim(PreclaimContext const& ctx) return invoke_preclaim(ctx); case ttREMIT: return invoke_preclaim(ctx); + case ttREMARKS_SET: + return invoke_preclaim(ctx); case ttURITOKEN_MINT: case ttURITOKEN_BURN: case ttURITOKEN_BUY: @@ -373,6 +378,8 @@ invoke_calculateBaseFee(ReadView const& view, STTx const& tx) return Invoke::calculateBaseFee(view, tx); case ttREMIT: return Remit::calculateBaseFee(view, tx); + case ttREMARKS_SET: + return SetRemarks::calculateBaseFee(view, tx); case ttURITOKEN_MINT: case ttURITOKEN_BURN: case ttURITOKEN_BUY: @@ -556,6 +563,10 @@ invoke_apply(ApplyContext& ctx) Remit p(ctx); return p(); } + case ttREMARKS_SET: { + SetRemarks p(ctx); + return p(); + } case ttURITOKEN_MINT: case ttURITOKEN_BURN: case ttURITOKEN_BUY: diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 7661b962c..3764b3b78 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -74,7 +74,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 = 80; +static constexpr std::size_t numFeatures = 81; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated @@ -362,6 +362,7 @@ extern uint256 const fix240819; extern uint256 const fixPageCap; extern uint256 const fix240911; extern uint256 const fixFloatDivide; +extern uint256 const featureRemarks; extern uint256 const featureTouch; extern uint256 const fixReduceImport; extern uint256 const fixXahauV3; diff --git a/src/ripple/protocol/LedgerFormats.h b/src/ripple/protocol/LedgerFormats.h index 6134a8f33..56321df7a 100644 --- a/src/ripple/protocol/LedgerFormats.h +++ b/src/ripple/protocol/LedgerFormats.h @@ -315,6 +315,9 @@ enum LedgerSpecificFlags { // ltURI_TOKEN lsfBurnable = 0x00000001, // True, issuer can burn the token + + // remarks + lsfImmutable = 1, }; //------------------------------------------------------------------------------ diff --git a/src/ripple/protocol/SField.h b/src/ripple/protocol/SField.h index 56aed3a5a..ae1c1007a 100644 --- a/src/ripple/protocol/SField.h +++ b/src/ripple/protocol/SField.h @@ -460,6 +460,7 @@ extern SF_UINT256 const sfNFTokenID; extern SF_UINT256 const sfEmitParentTxnID; extern SF_UINT256 const sfEmitNonce; extern SF_UINT256 const sfEmitHookHash; +extern SF_UINT256 const sfObjectID; // 256-bit (uncommon) extern SF_UINT256 const sfBookDirectory; @@ -539,6 +540,8 @@ extern SF_VL const sfHookReturnString; extern SF_VL const sfHookParameterName; extern SF_VL const sfHookParameterValue; extern SF_VL const sfBlob; +extern SF_VL const sfRemarkName; +extern SF_VL const sfRemarkValue; // account extern SF_ACCOUNT const sfAccount; @@ -596,6 +599,7 @@ extern SField const sfImportVLKey; extern SField const sfHookEmission; extern SField const sfMintURIToken; extern SField const sfAmountEntry; +extern SField const sfRemark; // array of objects (common) // ARRAY/1 is reserved for end of array @@ -624,6 +628,7 @@ extern SField const sfActiveValidators; extern SField const sfImportVLKeys; extern SField const sfHookEmissions; extern SField const sfAmounts; +extern SField const sfRemarks; //------------------------------------------------------------------------------ diff --git a/src/ripple/protocol/TER.h b/src/ripple/protocol/TER.h index 7cd3cae42..31ad16a3c 100644 --- a/src/ripple/protocol/TER.h +++ b/src/ripple/protocol/TER.h @@ -341,6 +341,8 @@ enum TECcodes : TERUnderlyingType { tecXCHAIN_SELF_COMMIT = 185, // RESERVED - XCHAIN tecXCHAIN_BAD_PUBLIC_KEY_ACCOUNT_PAIR = 186, // RESERVED - XCHAIN tecINSUF_RESERVE_SELLER = 187, + tecIMMUTABLE = 188, + tecTOO_MANY_REMARKS = 189, tecLAST_POSSIBLE_ENTRY = 255, }; diff --git a/src/ripple/protocol/TxFlags.h b/src/ripple/protocol/TxFlags.h index 24b58b596..2f660ff80 100644 --- a/src/ripple/protocol/TxFlags.h +++ b/src/ripple/protocol/TxFlags.h @@ -191,6 +191,9 @@ enum ClaimRewardFlags : uint32_t { }; constexpr std::uint32_t const tfClaimRewardMask = ~(tfUniversal | tfOptOut); +// Remarks flags: +constexpr std::uint32_t const tfImmutable = 1; + // clang-format on } // namespace ripple diff --git a/src/ripple/protocol/TxFormats.h b/src/ripple/protocol/TxFormats.h index 2f287efe5..7fdd647de 100644 --- a/src/ripple/protocol/TxFormats.h +++ b/src/ripple/protocol/TxFormats.h @@ -146,6 +146,9 @@ enum TxType : std::uint16_t ttURITOKEN_CREATE_SELL_OFFER = 48, ttURITOKEN_CANCEL_SELL_OFFER = 49, + /* A note attaching transactor that allows the owner or issuer (on a object by object basis) to attach remarks */ + ttREMARKS_SET = 94, + /* A payment transactor that delivers only the exact amounts specified, creating accounts and TLs as needed * that the sender pays for. */ ttREMIT = 95, diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 35ada0af7..68767da68 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -468,6 +468,7 @@ REGISTER_FIX (fix240819, Supported::yes, VoteBehavior::De REGISTER_FIX (fixPageCap, Supported::yes, VoteBehavior::DefaultYes); REGISTER_FIX (fix240911, Supported::yes, VoteBehavior::DefaultYes); REGISTER_FIX (fixFloatDivide, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FEATURE(Remarks, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FEATURE(Touch, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FIX (fixReduceImport, Supported::yes, VoteBehavior::DefaultYes); REGISTER_FIX (fixXahauV3, Supported::yes, VoteBehavior::DefaultYes); diff --git a/src/ripple/protocol/impl/InnerObjectFormats.cpp b/src/ripple/protocol/impl/InnerObjectFormats.cpp index 99f826651..4c2500ff2 100644 --- a/src/ripple/protocol/impl/InnerObjectFormats.cpp +++ b/src/ripple/protocol/impl/InnerObjectFormats.cpp @@ -159,6 +159,14 @@ InnerObjectFormats::InnerObjectFormats() {sfDigest, soeOPTIONAL}, {sfFlags, soeOPTIONAL}, }); + + add(sfRemark.jsonName.c_str(), + sfRemark.getCode(), + { + {sfRemarkName, soeREQUIRED}, + {sfRemarkValue, soeOPTIONAL}, + {sfFlags, soeOPTIONAL}, + }); } InnerObjectFormats const& diff --git a/src/ripple/protocol/impl/LedgerFormats.cpp b/src/ripple/protocol/impl/LedgerFormats.cpp index 02b381d33..ef811667c 100644 --- a/src/ripple/protocol/impl/LedgerFormats.cpp +++ b/src/ripple/protocol/impl/LedgerFormats.cpp @@ -31,6 +31,7 @@ LedgerFormats::LedgerFormats() {sfLedgerIndex, soeOPTIONAL}, {sfLedgerEntryType, soeREQUIRED}, {sfFlags, soeREQUIRED}, + {sfRemarks, soeOPTIONAL}, }; add(jss::AccountRoot, diff --git a/src/ripple/protocol/impl/SField.cpp b/src/ripple/protocol/impl/SField.cpp index 8b8050351..6d205adbe 100644 --- a/src/ripple/protocol/impl/SField.cpp +++ b/src/ripple/protocol/impl/SField.cpp @@ -211,6 +211,7 @@ CONSTRUCT_TYPED_SFIELD(sfNFTokenID, "NFTokenID", UINT256, CONSTRUCT_TYPED_SFIELD(sfEmitParentTxnID, "EmitParentTxnID", UINT256, 11); CONSTRUCT_TYPED_SFIELD(sfEmitNonce, "EmitNonce", UINT256, 12); CONSTRUCT_TYPED_SFIELD(sfEmitHookHash, "EmitHookHash", UINT256, 13); +CONSTRUCT_TYPED_SFIELD(sfObjectID, "ObjectID", UINT256, 14); // 256-bit (uncommon) CONSTRUCT_TYPED_SFIELD(sfBookDirectory, "BookDirectory", UINT256, 16); @@ -292,6 +293,8 @@ CONSTRUCT_TYPED_SFIELD(sfHookReturnString, "HookReturnString", VL, CONSTRUCT_TYPED_SFIELD(sfHookParameterName, "HookParameterName", VL, 24); CONSTRUCT_TYPED_SFIELD(sfHookParameterValue, "HookParameterValue", VL, 25); CONSTRUCT_TYPED_SFIELD(sfBlob, "Blob", VL, 26); +CONSTRUCT_TYPED_SFIELD(sfRemarkValue, "RemarkValue", VL, 98); +CONSTRUCT_TYPED_SFIELD(sfRemarkName, "RemarkName", VL, 99); // account CONSTRUCT_TYPED_SFIELD(sfAccount, "Account", ACCOUNT, 1); @@ -346,6 +349,7 @@ CONSTRUCT_UNTYPED_SFIELD(sfHookExecution, "HookExecution", OBJECT, CONSTRUCT_UNTYPED_SFIELD(sfHookDefinition, "HookDefinition", OBJECT, 22); CONSTRUCT_UNTYPED_SFIELD(sfHookParameter, "HookParameter", OBJECT, 23); CONSTRUCT_UNTYPED_SFIELD(sfHookGrant, "HookGrant", OBJECT, 24); +CONSTRUCT_UNTYPED_SFIELD(sfRemark, "Remark", OBJECT, 97); CONSTRUCT_UNTYPED_SFIELD(sfGenesisMint, "GenesisMint", OBJECT, 96); CONSTRUCT_UNTYPED_SFIELD(sfActiveValidator, "ActiveValidator", OBJECT, 95); CONSTRUCT_UNTYPED_SFIELD(sfImportVLKey, "ImportVLKey", OBJECT, 94); @@ -372,6 +376,7 @@ CONSTRUCT_UNTYPED_SFIELD(sfDisabledValidators, "DisabledValidators", ARRAY, CONSTRUCT_UNTYPED_SFIELD(sfHookExecutions, "HookExecutions", ARRAY, 18); CONSTRUCT_UNTYPED_SFIELD(sfHookParameters, "HookParameters", ARRAY, 19); CONSTRUCT_UNTYPED_SFIELD(sfHookGrants, "HookGrants", ARRAY, 20); +CONSTRUCT_UNTYPED_SFIELD(sfRemarks, "Remarks", ARRAY, 97); CONSTRUCT_UNTYPED_SFIELD(sfGenesisMints, "GenesisMints", ARRAY, 96); CONSTRUCT_UNTYPED_SFIELD(sfActiveValidators, "ActiveValidators", ARRAY, 95); CONSTRUCT_UNTYPED_SFIELD(sfImportVLKeys, "ImportVLKeys", ARRAY, 94); diff --git a/src/ripple/protocol/impl/TER.cpp b/src/ripple/protocol/impl/TER.cpp index e41134a0c..30bcf46e2 100644 --- a/src/ripple/protocol/impl/TER.cpp +++ b/src/ripple/protocol/impl/TER.cpp @@ -92,6 +92,8 @@ transResults() MAKE_ERROR(tecREQUIRES_FLAG, "The transaction or part-thereof requires a flag that wasn't set."), MAKE_ERROR(tecPRECISION_LOSS, "The amounts used by the transaction cannot interact."), MAKE_ERROR(tecINSUF_RESERVE_SELLER, "The seller of an object has insufficient reserves, and thus cannot complete the sale."), + MAKE_ERROR(tecIMMUTABLE, "The remark is marked immutable on the object, and therefore cannot be updated."), + MAKE_ERROR(tecTOO_MANY_REMARKS, "The number of remarks on the object would exceed the limit of 32."), MAKE_ERROR(tefALREADY, "The exact transaction was already in this ledger."), MAKE_ERROR(tefBAD_ADD_AUTH, "Not authorized to add account."), MAKE_ERROR(tefBAD_AUTH, "Transaction's public key is not authorized."), diff --git a/src/ripple/protocol/impl/TxFormats.cpp b/src/ripple/protocol/impl/TxFormats.cpp index 6c38711ad..fb7d6e581 100644 --- a/src/ripple/protocol/impl/TxFormats.cpp +++ b/src/ripple/protocol/impl/TxFormats.cpp @@ -456,6 +456,14 @@ TxFormats::TxFormats() {sfTicketSequence, soeOPTIONAL}, }, commonFields); + + add(jss::SetRemarks, + ttREMARKS_SET, + { + {sfObjectID, soeREQUIRED}, + {sfRemarks, soeREQUIRED}, + }, + commonFields); } TxFormats const& diff --git a/src/ripple/protocol/jss.h b/src/ripple/protocol/jss.h index 961e63c44..0faa053f6 100644 --- a/src/ripple/protocol/jss.h +++ b/src/ripple/protocol/jss.h @@ -122,6 +122,7 @@ JSS(Remit); // transaction type. JSS(RippleState); // ledger type. JSS(SLE_hit_rate); // out: GetCounts. JSS(SetFee); // transaction type. +JSS(SetRemarks); // transaction type JSS(UNLModify); // transaction type. JSS(UNLReport); // transaction type. JSS(SettleDelay); // in: TransactionSign diff --git a/src/test/app/SetRemarks_test.cpp b/src/test/app/SetRemarks_test.cpp new file mode 100644 index 000000000..8948c4613 --- /dev/null +++ b/src/test/app/SetRemarks_test.cpp @@ -0,0 +1,591 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 XRPL-Labs + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include +#include +#include +#include +#include +#include +#include + +namespace ripple { +namespace test { +struct SetRemarks_test : public beast::unit_test::suite +{ + // debugRemarks(env, keylet::account(alice).key); + void + debugRemarks(jtx::Env& env, uint256 const& id) + { + Json::Value params; + params[jss::index] = strHex(id); + auto const info = env.rpc("json", "ledger_entry", to_string(params)); + std::cout << "INFO: " << info << "\n"; + } + + void + validateRemarks( + ReadView const& view, + uint256 const& id, + std::vector const& marks) + { + using namespace jtx; + auto const slep = view.read(keylet::unchecked(id)); + if (slep && slep->isFieldPresent(sfRemarks)) + { + auto const& remarksObj = slep->getFieldArray(sfRemarks); + BEAST_EXPECT(remarksObj.size() == marks.size()); + for (int i = 0; i < marks.size(); ++i) + { + remarks::remark const expectedMark = marks[i]; + STObject const remark = remarksObj[i]; + + Blob name = remark.getFieldVL(sfRemarkName); + // BEAST_EXPECT(expectedMark.name == name); + + uint32_t flags = remark.isFieldPresent(sfFlags) + ? remark.getFieldU32(sfFlags) + : 0; + BEAST_EXPECT(expectedMark.flags == flags); + + std::optional val; + if (remark.isFieldPresent(sfRemarkValue)) + val = remark.getFieldVL(sfRemarkValue); + // BEAST_EXPECT(expectedMark.value == val); + } + } + } + + void + testEnabled(FeatureBitset features) + { + testcase("enabled"); + using namespace jtx; + + // setup env + auto const alice = Account("alice"); + auto const bob = Account("bob"); + + for (bool const withRemarks : {false, true}) + { + // If the Remarks amendment is not enabled, you cannot add remarks + auto const amend = + withRemarks ? features : features - featureRemarks; + Env env{*this, amend}; + + env.fund(XRP(1000), alice, bob); + env.close(); + std::vector marks = { + {"CAFE", "DEADBEEF", 0}, + }; + auto const txResult = + withRemarks ? ter(tesSUCCESS) : ter(temDISABLED); + env(remarks::setRemarks(alice, keylet::account(alice).key, marks), + fee(XRP(1)), + txResult); + env.close(); + } + } + + void + testPreflightInvalid(FeatureBitset features) + { + testcase("preflight invalid"); + using namespace jtx; + + // setup env + auto const alice = Account("alice"); + auto const bob = Account("bob"); + + Env env{*this, features}; + + env.fund(XRP(1000), alice, bob); + env.close(); + + //---------------------------------------------------------------------- + // preflight + + // temDISABLED + // DA: testEnabled() + + // temINVALID_FLAG: SetRemarks: Invalid flags set. + { + std::vector marks = { + {"CAFE", "DEADBEEF", 0}, + }; + env(remarks::setRemarks(alice, keylet::account(alice).key, marks), + txflags(tfClose), + fee(XRP(1)), + ter(temINVALID_FLAG)); + env.close(); + } + // temMALFORMED: SetRemarks: Cannot set more than 32 remarks (or fewer + // than 1) in a txn. + { + std::vector marks; + for (int i = 0; i < 0; ++i) + { + marks.push_back({"CAFE", "DEADBEEF", 0}); + } + env(remarks::setRemarks(alice, keylet::account(alice).key, marks), + fee(XRP(1)), + ter(temMALFORMED)); + env.close(); + } + // temMALFORMED: SetRemarks: Cannot set more than 32 remarks (or fewer + // than 1) in a txn. + { + std::vector marks; + for (int i = 0; i < 33; ++i) + { + marks.push_back({"CAFE", "DEADBEEF", 0}); + } + env(remarks::setRemarks(alice, keylet::account(alice).key, marks), + fee(XRP(1)), + ter(temMALFORMED)); + env.close(); + } + // temMALFORMED: SetRemarks: contained non-sfRemark field. + { + std::vector marks = { + {"CAFE", "DEADBEEF", 0}, + }; + Json::Value jv; + jv[jss::TransactionType] = jss::SetRemarks; + jv[jss::Account] = alice.human(); + jv[sfObjectID.jsonName] = strHex(keylet::account(alice).key); + auto& ja = jv[sfRemarks.getJsonName()]; + for (std::size_t i = 0; i < 1; ++i) + { + ja[i][sfGenesisMint.jsonName] = Json::Value{}; + ja[i][sfGenesisMint.jsonName][jss::Amount] = + STAmount(1).getJson(JsonOptions::none); + ja[i][sfGenesisMint.jsonName][jss::Destination] = bob.human(); + } + jv[sfRemarks.jsonName] = ja; + env(jv, fee(XRP(1)), ter(temMALFORMED)); + env.close(); + } + // temMALFORMED: SetRemarks: duplicate RemarkName entry. + { + std::vector marks = { + {"CAFE", "DEADBEEF", 0}, + {"CAFE", "DEADBEEF", 0}, + }; + env(remarks::setRemarks(alice, keylet::account(alice).key, marks), + fee(XRP(1)), + ter(temMALFORMED)); + env.close(); + } + // temMALFORMED: SetRemarks: RemarkName cannot be empty or larger than + // 256 chars. + { + std::vector marks = { + {"", "DEADBEEF", 0}, + }; + env(remarks::setRemarks(alice, keylet::account(alice).key, marks), + fee(XRP(1)), + ter(temMALFORMED)); + env.close(); + } + // temMALFORMED: SetRemarks: RemarkName cannot be empty or larger than + // 256 chars. + { + std::string const name((256 * 2) + 1, 'A'); + std::vector marks = { + {name, "DEADBEEF", 0}, + }; + env(remarks::setRemarks(alice, keylet::account(alice).key, marks), + fee(XRP(1)), + ter(temMALFORMED)); + env.close(); + } + // temMALFORMED: SetRemarks: Flags must be either tfImmutable or 0 + { + std::vector marks = { + {"CAFE", "DEADBEEF", 2}, + }; + env(remarks::setRemarks(alice, keylet::account(alice).key, marks), + fee(XRP(1)), + ter(temMALFORMED)); + env.close(); + } + // temMALFORMED: SetRemarks: A remark deletion cannot be marked + // immutable. + { + std::vector marks = { + {"CAFE", std::nullopt, 1}, + }; + env(remarks::setRemarks(alice, keylet::account(alice).key, marks), + fee(XRP(1)), + ter(temMALFORMED)); + env.close(); + } + // temMALFORMED: SetRemarks: RemarkValue cannot be empty or larger than + // 256 chars. + { + std::vector marks = { + {"CAFE", "", 0}, + }; + env(remarks::setRemarks(alice, keylet::account(alice).key, marks), + fee(XRP(1)), + ter(temMALFORMED)); + env.close(); + } + // temMALFORMED: SetRemarks: RemarkValue cannot be empty or larger than + // 256 chars. + { + std::string const value((256 * 2) + 1, 'A'); + std::vector marks = { + {"CAFE", value, 0}, + }; + env(remarks::setRemarks(alice, keylet::account(alice).key, marks), + fee(XRP(1)), + ter(temMALFORMED)); + env.close(); + } + } + + void + testPreclaimInvalid(FeatureBitset features) + { + testcase("preclaim invalid"); + using namespace jtx; + + // setup env + Env env{*this, features}; + auto const alice = Account("alice"); + auto const bob = Account("bob"); + auto const carol = Account("carol"); + env.memoize(carol); + env.fund(XRP(1000), alice, bob); + env.close(); + + std::vector marks = { + {"CAFE", "DEADBEEF", 0}, + }; + + //---------------------------------------------------------------------- + // preclaim + + // temDISABLED + // DA: testEnabled() + + // terNO_ACCOUNT - account doesnt exist + { + auto const carol = Account("carol"); + env.memoize(carol); + auto tx = + remarks::setRemarks(carol, keylet::account(carol).key, marks); + tx[jss::Sequence] = 0; + env(tx, carol, fee(XRP(1)), ter(terNO_ACCOUNT)); + env.close(); + } + + // tecNO_TARGET - object doesnt exist + { + env(remarks::setRemarks(alice, keylet::account(carol).key, marks), + fee(XRP(1)), + ter(tecNO_TARGET)); + env.close(); + } + + // tecNO_PERMISSION: !issuer + { + env(deposit::auth(bob, alice)); + env(remarks::setRemarks( + alice, keylet::depositPreauth(bob, alice).key, marks), + fee(XRP(1)), + ter(tecNO_PERMISSION)); + env.close(); + } + // tecNO_PERMISSION: issuer != _account + { + env(remarks::setRemarks(alice, keylet::account(bob).key, marks), + fee(XRP(1)), + ter(tecNO_PERMISSION)); + env.close(); + } + // tecIMMUTABLE: SetRemarks: attempt to mutate an immutable remark. + { + // alice creates immutable remark + std::vector immutableMarks = { + {"CAFF", "DEAD", tfImmutable}, + }; + env(remarks::setRemarks( + alice, keylet::account(alice).key, immutableMarks), + fee(XRP(1)), + ter(tesSUCCESS)); + env.close(); + + // alice cannot update immutable remark + std::vector badMarks = { + {"CAFF", "DEADBEEF", 0}, + }; + env(remarks::setRemarks( + alice, keylet::account(alice).key, badMarks), + fee(XRP(1)), + ter(tecIMMUTABLE)); + env.close(); + } + // tecCLAIM: SetRemarks: insane remarks accounting. + {} // tecTOO_MANY_REMARKS: SetRemarks: an object may have at most 32 + // remarks. + { + std::vector _marks; + unsigned int hexValue = 0xEFAC; + for (int i = 0; i < 31; ++i) + { + std::stringstream ss; + ss << std::hex << std::uppercase << hexValue; + _marks.push_back({ss.str(), "DEADBEEF", 0}); + hexValue++; + } + env(remarks::setRemarks(alice, keylet::account(alice).key, _marks), + fee(XRP(1)), + ter(tesSUCCESS)); + env.close(); + env(remarks::setRemarks(alice, keylet::account(alice).key, marks), + fee(XRP(1)), + ter(tecTOO_MANY_REMARKS)); + env.close(); + } + } + + void + testDoApplyInvalid(FeatureBitset features) + { + testcase("doApply invalid"); + using namespace jtx; + + //---------------------------------------------------------------------- + // doApply + + // terNO_ACCOUNT + // tecNO_TARGET + // tecNO_PERMISSION + // tecTOO_MANY_REMARKS + } + + void + testDelete(FeatureBitset features) + { + testcase("delete"); + using namespace jtx; + + // setup env + Env env{*this, features}; + auto const alice = Account("alice"); + auto const bob = Account("bob"); + env.fund(XRP(1000), alice, bob); + env.close(); + + auto const id = keylet::account(alice).key; + + // Set Remarks + { + std::vector marks = { + {"CAFE", "DEADBEEF", 0}, + }; + env(remarks::setRemarks(alice, id, marks), fee(XRP(1))); + env.close(); + validateRemarks(*env.current(), id, marks); + } + + // Delete Remarks + { + std::vector marks = { + {"CAFE", std::nullopt, 0}, + }; + env(remarks::setRemarks(alice, id, marks), fee(XRP(1))); + env.close(); + validateRemarks(*env.current(), id, {}); + } + } + + void + testLedgerObjects(FeatureBitset features) + { + testcase("ledger objects"); + using namespace jtx; + + // setup env + Env env{*this, features}; + auto const alice = Account("alice"); + auto const bob = Account("bob"); + auto const gw = Account("gw"); + auto const USD = gw["USD"]; + env.fund(XRP(10000), alice, bob, gw); + env.close(); + env.trust(USD(10000), alice); + env.trust(USD(10000), bob); + env.close(); + env(pay(gw, alice, USD(1000))); + env(pay(gw, bob, USD(1000))); + env.close(); + + std::vector marks = { + {"CAFE", "DEADBEEF", 0}, + }; + + // ltACCOUNT_ROOT + { + auto const id = keylet::account(alice).key; + env(remarks::setRemarks(alice, id, marks), fee(XRP(1))); + env.close(); + validateRemarks(*env.current(), id, marks); + } + // ltOFFER + { + auto const id = keylet::offer(alice, env.seq(alice)).key; + env(offer(alice, XRP(10), USD(10)), fee(XRP(1))); + env(remarks::setRemarks(alice, id, marks), fee(XRP(1))); + env.close(); + validateRemarks(*env.current(), id, marks); + } + // ltESCROW + { + using namespace std::literals::chrono_literals; + auto const id = keylet::escrow(alice, env.seq(alice)).key; + env(escrow::create(alice, bob, XRP(10)), + escrow::finish_time(env.now() + 1s), + fee(XRP(1))); + env(remarks::setRemarks(alice, id, marks), fee(XRP(1))); + env.close(); + validateRemarks(*env.current(), id, marks); + } + // ltTICKET + { + auto const id = keylet::ticket(alice, env.seq(alice) + 1).key; + env(ticket::create(alice, 10), fee(XRP(1))); + env(remarks::setRemarks(alice, id, marks), fee(XRP(1))); + env.close(); + validateRemarks(*env.current(), id, marks); + } + // ltPAYCHAN + { + using namespace std::literals::chrono_literals; + auto const id = keylet::payChan(alice, bob, env.seq(alice)).key; + auto const pk = alice.pk(); + auto const settleDelay = 100s; + env(paychan::create(alice, bob, XRP(10), settleDelay, pk), + fee(XRP(1))); + env(remarks::setRemarks(alice, id, marks), fee(XRP(1))); + env.close(); + validateRemarks(*env.current(), id, marks); + } + // ltCHECK + { + auto const id = keylet::check(alice, env.seq(alice)).key; + env(check::create(alice, bob, XRP(10)), fee(XRP(1))); + env(remarks::setRemarks(alice, id, marks), fee(XRP(1))); + env.close(); + validateRemarks(*env.current(), id, marks); + } + // ltDEPOSIT_PREAUTH + { + env(fset(bob, asfDepositAuth)); + auto const id = keylet::depositPreauth(alice, bob).key; + env(deposit::auth(alice, bob), fee(XRP(1))); + env(remarks::setRemarks(alice, id, marks), fee(XRP(1))); + env.close(); + validateRemarks(*env.current(), id, marks); + } + // ltURI_TOKEN + { + std::string const uri(256, 'A'); + auto const id = + keylet::uritoken(alice, Blob(uri.begin(), uri.end())).key; + env(uritoken::mint(alice, uri), fee(XRP(1))); + env(remarks::setRemarks(alice, id, marks), fee(XRP(1))); + env.close(); + validateRemarks(*env.current(), id, marks); + } + // ltRIPPLE_STATE: bal < 0 + { + auto const alice2 = Account("alice2"); + env.fund(XRP(1000), alice2); + env.close(); + env.trust(USD(10000), alice2); + auto const id = keylet::line(alice2, USD).key; + env(pay(gw, alice2, USD(1000))); + env(remarks::setRemarks(gw, id, marks), fee(XRP(1))); + env.close(); + validateRemarks(*env.current(), id, marks); + } + // ltRIPPLE_STATE: bal > 0 + { + auto const carol0 = Account("carol0"); + env.fund(XRP(1000), carol0); + env.close(); + env.trust(USD(10000), carol0); + auto const id = keylet::line(carol0, USD).key; + env(pay(gw, carol0, USD(1000))); + env(remarks::setRemarks(gw, id, marks), fee(XRP(1))); + env.close(); + validateRemarks(*env.current(), id, marks); + } + // ltRIPPLE_STATE: highReserve + { + auto const dan1 = Account("dan1"); + env.fund(XRP(1000), dan1); + env.close(); + env.trust(USD(1000), dan1); + auto const id = keylet::line(dan1, USD).key; + env(remarks::setRemarks(gw, id, marks), fee(XRP(1))); + env.close(); + validateRemarks(*env.current(), id, marks); + } + // ltRIPPLE_STATE: lowReserve + { + auto const bob0 = Account("bob0"); + env.fund(XRP(1000), bob0); + env.close(); + env.trust(USD(1000), bob0); + auto const id = keylet::line(bob0, USD).key; + env(remarks::setRemarks(gw, id, marks), fee(XRP(1))); + env.close(); + validateRemarks(*env.current(), id, marks); + } + } + + void + testWithFeats(FeatureBitset features) + { + testEnabled(features); + testPreflightInvalid(features); + testPreclaimInvalid(features); + testDoApplyInvalid(features); + testDelete(features); + testLedgerObjects(features); + } + +public: + void + run() override + { + using namespace test::jtx; + auto const sa = supported_amendments(); + testWithFeats(sa); + } +}; + +BEAST_DEFINE_TESTSUITE(SetRemarks, app, ripple); +} // namespace test +} // namespace ripple diff --git a/src/test/jtx.h b/src/test/jtx.h index a91ae3b2f..8d9b53bfe 100644 --- a/src/test/jtx.h +++ b/src/test/jtx.h @@ -57,6 +57,7 @@ #include #include #include +#include #include #include #include diff --git a/src/test/jtx/impl/remarks.cpp b/src/test/jtx/impl/remarks.cpp new file mode 100644 index 000000000..ad49afb3b --- /dev/null +++ b/src/test/jtx/impl/remarks.cpp @@ -0,0 +1,56 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2023 XRPL Labs + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include +#include + +namespace ripple { +namespace test { +namespace jtx { +namespace remarks { + +Json::Value +setRemarks( + jtx::Account const& account, + uint256 const& id, + std::vector const& marks) +{ + using namespace jtx; + Json::Value jv; + jv[jss::TransactionType] = jss::SetRemarks; + jv[jss::Account] = account.human(); + jv[sfObjectID.jsonName] = strHex(id); + auto& ja = jv[sfRemarks.getJsonName()]; + for (std::size_t i = 0; i < marks.size(); ++i) + { + ja[i][sfRemark.jsonName] = Json::Value{}; + ja[i][sfRemark.jsonName][sfRemarkName.jsonName] = marks[i].name; + if (marks[i].value) + ja[i][sfRemark.jsonName][sfRemarkValue.jsonName] = *marks[i].value; + if (marks[i].flags) + ja[i][sfRemark.jsonName][sfFlags.jsonName] = *marks[i].flags; + } + jv[sfRemarks.jsonName] = ja; + return jv; +} + +} // namespace remarks +} // namespace jtx +} // namespace test +} // namespace ripple diff --git a/src/test/jtx/remarks.h b/src/test/jtx/remarks.h new file mode 100644 index 000000000..b4d958598 --- /dev/null +++ b/src/test/jtx/remarks.h @@ -0,0 +1,64 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 XRPL Labs + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#ifndef RIPPLE_TEST_JTX_REMARKS_H_INCLUDED +#define RIPPLE_TEST_JTX_REMARKS_H_INCLUDED + +#include +#include + +namespace ripple { +namespace test { +namespace jtx { + +namespace remarks { + +struct remark +{ + std::string name; + std::optional value; + std::optional flags; + remark( + std::string name_, + std::optional value_ = std::nullopt, + std::optional flags_ = std::nullopt) + : name(name_), value(value_), flags(flags_) + { + if (value_) + value = *value_; + + if (flags_) + flags = *flags_; + } +}; + +Json::Value +setRemarks( + jtx::Account const& account, + uint256 const& id, + std::vector const& marks); + +} // namespace remarks + +} // namespace jtx + +} // namespace test +} // namespace ripple + +#endif // RIPPLE_TEST_JTX_REMARKS_H_INCLUDED \ No newline at end of file From 658eb73e00c6e89841cc771154e976f6c641d38b Mon Sep 17 00:00:00 2001 From: Denis Angell Date: Wed, 16 Apr 2025 11:29:35 +0200 Subject: [PATCH 3/5] build: add interface library libxrpl: (#4449) Make it easy for projects to depend on libxrpl by adding an `ALIAS` target named `xrpl::libxrpl` for projects to link. The name was chosen because: * The current library target is named `xrpl_core`. There is no other "non-core" library target against which we need to distinguish the "core" library. We only export one library target, and it should just be named after the project to keep things simple and predictable. * Underscores in target or library names are generally discouraged. * Every target exported in CMake should be prefixed with the project name. By adding an `ALIAS` target, existing consumers who use the `xrpl_core` target will not be affected. * In the future, there can be a migration plan to make `xrpl_core` the `ALIAS` target (and `libxrpl` the "real" target, which will affect the filename of the compiled binary), and eventually remove it entirely. Also: * Fix the Conan recipe so that consumers using Conan import a target named `xrpl::libxrpl`. This way, every consumer can use the same instructions. * Document the two easiest methods to depend on libxrpl. Both have been tested. * See #4443. --- Builds/CMake/RippledCore.cmake | 5 +++++ conanfile.py | 13 +++++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index 142409bc0..92593d538 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -23,6 +23,11 @@ else() message(STATUS "ACL not found, continuing without ACL support") endif() +add_library(libxrpl INTERFACE) +target_link_libraries(libxrpl INTERFACE xrpl_core) +add_library(xrpl::libxrpl ALIAS libxrpl) + + #[===============================[ beast/legacy FILES: TODO: review these sources for removal or replacement diff --git a/conanfile.py b/conanfile.py index b27b769c8..9dea082ff 100644 --- a/conanfile.py +++ b/conanfile.py @@ -1,4 +1,4 @@ -from conans import ConanFile +from conan import ConanFile from conan.tools.cmake import CMake, CMakeToolchain, cmake_layout import re @@ -109,7 +109,9 @@ class Xrpl(ConanFile): if self.options.rocksdb: self.requires('rocksdb/6.27.3') - exports_sources = 'CMakeLists.txt', 'Builds/CMake/*', 'src/*', 'cfg/*' + exports_sources = ( + 'CMakeLists.txt', 'Builds/*', 'bin/getRippledInfo', 'src/*', 'cfg/*' + ) def layout(self): cmake_layout(self) @@ -143,8 +145,11 @@ class Xrpl(ConanFile): cmake.install() def package_info(self): - self.cpp_info.libs = [ + libxrpl = self.cpp_info.components['libxrpl'] + libxrpl.libs = [ 'libxrpl_core.a', - 'libed25519-donna.a', + 'libed25519.a', 'libsecp256k1.a', ] + libxrpl.includedirs = ['include'] + libxrpl.requires = ['boost::boost'] From 989532702d198165b8ddf803bafc450026819b29 Mon Sep 17 00:00:00 2001 From: tequ Date: Thu, 17 Apr 2025 15:16:59 +0900 Subject: [PATCH 4/5] Update clang-format workflow (#490) --- .github/workflows/clang-format.yml | 31 ++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/.github/workflows/clang-format.yml b/.github/workflows/clang-format.yml index 52432adda..00f860dec 100644 --- a/.github/workflows/clang-format.yml +++ b/.github/workflows/clang-format.yml @@ -4,21 +4,32 @@ on: [push, pull_request] jobs: check: - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 env: CLANG_VERSION: 10 steps: - uses: actions/checkout@v3 - - name: Install clang-format + # - name: Install clang-format + # run: | + # codename=$( lsb_release --codename --short ) + # sudo tee /etc/apt/sources.list.d/llvm.list >/dev/null </dev/null < Date: Thu, 17 Apr 2025 08:40:27 +0200 Subject: [PATCH 5/5] fix conan --- src/ripple/app/tx/impl/SetRemarks.cpp | 1 + src/test/app/SetRemarks_test.cpp | 6 ++---- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/ripple/app/tx/impl/SetRemarks.cpp b/src/ripple/app/tx/impl/SetRemarks.cpp index f45479bf5..34879baac 100644 --- a/src/ripple/app/tx/impl/SetRemarks.cpp +++ b/src/ripple/app/tx/impl/SetRemarks.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include namespace ripple { diff --git a/src/test/app/SetRemarks_test.cpp b/src/test/app/SetRemarks_test.cpp index 8948c4613..82b782035 100644 --- a/src/test/app/SetRemarks_test.cpp +++ b/src/test/app/SetRemarks_test.cpp @@ -377,10 +377,8 @@ struct SetRemarks_test : public beast::unit_test::suite //---------------------------------------------------------------------- // doApply - // terNO_ACCOUNT - // tecNO_TARGET - // tecNO_PERMISSION - // tecTOO_MANY_REMARKS + // All checks in doApply are done in preclaim. + BEAST_EXPECT(1); } void