diff --git a/.github/scripts/levelization/README.md b/.github/scripts/levelization/README.md index 31c6d34b6b..f3ba1e2518 100644 --- a/.github/scripts/levelization/README.md +++ b/.github/scripts/levelization/README.md @@ -72,15 +72,15 @@ It generates many files of [results](results): desired as described above. In a perfect repo, this file will be empty. This file is committed to the repo, and is used by the [levelization - Github workflow](../../workflows/check-levelization.yml) to validate + Github workflow](../../workflows/reusable-check-levelization.yml) to validate that nothing changed. - [`ordering.txt`](results/ordering.txt): A list showing relationships between modules where there are no loops as they actually exist, as opposed to how they are desired as described above. This file is committed to the repo, and is used by the [levelization - Github workflow](../../workflows/check-levelization.yml) to validate + Github workflow](../../workflows/reusable-check-levelization.yml) to validate that nothing changed. -- [`levelization.yml`](../../workflows/check-levelization.yml) +- [`levelization.yml`](../../workflows/reusable-check-levelization.yml) Github Actions workflow to test that levelization loops haven't changed. Unfortunately, if changes are detected, it can't tell if they are improvements or not, so if you have resolved any issues or diff --git a/.github/scripts/levelization/results/ordering.txt b/.github/scripts/levelization/results/ordering.txt index 13de36e2a5..55df4c2672 100644 --- a/.github/scripts/levelization/results/ordering.txt +++ b/.github/scripts/levelization/results/ordering.txt @@ -138,6 +138,7 @@ test.toplevel > test.csf test.toplevel > xrpl.json test.unit_test > xrpl.basics tests.libxrpl > xrpl.basics +tests.libxrpl > xrpl.net xrpl.json > xrpl.basics xrpl.ledger > xrpl.basics xrpl.ledger > xrpl.protocol diff --git a/.github/workflows/on-pr.yml b/.github/workflows/on-pr.yml index 9befd31e71..a206bbf041 100644 --- a/.github/workflows/on-pr.yml +++ b/.github/workflows/on-pr.yml @@ -50,8 +50,8 @@ jobs: files: | # These paths are unique to `on-pr.yml`. .github/scripts/levelization/** - .github/workflows/check-levelization.yml - .github/workflows/notify-clio.yml + .github/workflows/reusable-check-levelization.yml + .github/workflows/reusable-notify-clio.yml .github/workflows/on-pr.yml # Keep the paths below in sync with those in `on-trigger.yml`. @@ -59,7 +59,7 @@ jobs: .github/actions/build-test/** .github/actions/setup-conan/** .github/scripts/strategy-matrix/** - .github/workflows/build-test.yml + .github/workflows/reusable-build-test.yml .github/workflows/reusable-strategy-matrix.yml .codecov.yml cmake/** @@ -93,12 +93,12 @@ jobs: check-levelization: needs: should-run if: ${{ needs.should-run.outputs.go == 'true' }} - uses: ./.github/workflows/check-levelization.yml + uses: ./.github/workflows/reusable-check-levelization.yml build-test: needs: should-run if: ${{ needs.should-run.outputs.go == 'true' }} - uses: ./.github/workflows/build-test.yml + uses: ./.github/workflows/reusable-build-test.yml strategy: matrix: os: [linux, macos, windows] @@ -112,7 +112,7 @@ jobs: - should-run - build-test if: ${{ needs.should-run.outputs.go == 'true' && contains(fromJSON('["release", "master"]'), github.ref_name) }} - uses: ./.github/workflows/notify-clio.yml + uses: ./.github/workflows/reusable-notify-clio.yml secrets: clio_notify_token: ${{ secrets.CLIO_NOTIFY_TOKEN }} conan_remote_username: ${{ secrets.CONAN_REMOTE_USERNAME }} diff --git a/.github/workflows/on-trigger.yml b/.github/workflows/on-trigger.yml index 06abbd3f17..7b5bda021f 100644 --- a/.github/workflows/on-trigger.yml +++ b/.github/workflows/on-trigger.yml @@ -14,7 +14,7 @@ on: - master paths: # These paths are unique to `on-trigger.yml`. - - ".github/workflows/check-missing-commits.yml" + - ".github/workflows/reusable-check-missing-commits.yml" - ".github/workflows/on-trigger.yml" - ".github/workflows/publish-docs.yml" @@ -23,7 +23,7 @@ on: - ".github/actions/build-test/**" - ".github/actions/setup-conan/**" - ".github/scripts/strategy-matrix/**" - - ".github/workflows/build-test.yml" + - ".github/workflows/reusable-build-test.yml" - ".github/workflows/reusable-strategy-matrix.yml" - ".codecov.yml" - "cmake/**" @@ -71,10 +71,10 @@ defaults: jobs: check-missing-commits: if: ${{ github.event_name == 'push' && github.ref_type == 'branch' && contains(fromJSON('["develop", "release"]'), github.ref_name) }} - uses: ./.github/workflows/check-missing-commits.yml + uses: ./.github/workflows/reusable-check-missing-commits.yml build-test: - uses: ./.github/workflows/build-test.yml + uses: ./.github/workflows/reusable-build-test.yml strategy: matrix: os: [linux, macos, windows] diff --git a/.github/workflows/build-test.yml b/.github/workflows/reusable-build-test.yml similarity index 100% rename from .github/workflows/build-test.yml rename to .github/workflows/reusable-build-test.yml diff --git a/.github/workflows/check-levelization.yml b/.github/workflows/reusable-check-levelization.yml similarity index 100% rename from .github/workflows/check-levelization.yml rename to .github/workflows/reusable-check-levelization.yml diff --git a/.github/workflows/check-missing-commits.yml b/.github/workflows/reusable-check-missing-commits.yml similarity index 100% rename from .github/workflows/check-missing-commits.yml rename to .github/workflows/reusable-check-missing-commits.yml diff --git a/.github/workflows/notify-clio.yml b/.github/workflows/reusable-notify-clio.yml similarity index 100% rename from .github/workflows/notify-clio.yml rename to .github/workflows/reusable-notify-clio.yml diff --git a/include/xrpl/protocol/Permissions.h b/include/xrpl/protocol/Permissions.h index 2eca441124..d3f5253cd0 100644 --- a/include/xrpl/protocol/Permissions.h +++ b/include/xrpl/protocol/Permissions.h @@ -86,6 +86,9 @@ public: std::optional getGranularTxType(GranularPermissionType const& gpType) const; + std::optional> const + getTxFeature(TxType txType) const; + bool isDelegatable(std::uint32_t const& permissionValue, Rules const& rules) const; diff --git a/include/xrpl/protocol/SField.h b/include/xrpl/protocol/SField.h index 2f85cf3b7c..b6ae98b48f 100644 --- a/include/xrpl/protocol/SField.h +++ b/include/xrpl/protocol/SField.h @@ -72,8 +72,10 @@ class STCurrency; STYPE(STI_VL, 7) \ STYPE(STI_ACCOUNT, 8) \ STYPE(STI_NUMBER, 9) \ + STYPE(STI_INT32, 10) \ + STYPE(STI_INT64, 11) \ \ - /* 10-13 are reserved */ \ + /* 12-13 are reserved */ \ STYPE(STI_OBJECT, 14) \ STYPE(STI_ARRAY, 15) \ \ @@ -356,6 +358,9 @@ using SF_UINT256 = TypedField>; using SF_UINT384 = TypedField>; using SF_UINT512 = TypedField>; +using SF_INT32 = TypedField>; +using SF_INT64 = TypedField>; + using SF_ACCOUNT = TypedField; using SF_AMOUNT = TypedField; using SF_ISSUE = TypedField; diff --git a/include/xrpl/protocol/STInteger.h b/include/xrpl/protocol/STInteger.h index b259638774..154ee7f203 100644 --- a/include/xrpl/protocol/STInteger.h +++ b/include/xrpl/protocol/STInteger.h @@ -81,6 +81,8 @@ using STUInt16 = STInteger; using STUInt32 = STInteger; using STUInt64 = STInteger; +using STInt32 = STInteger; + template inline STInteger::STInteger(Integer v) : value_(v) { diff --git a/include/xrpl/protocol/STObject.h b/include/xrpl/protocol/STObject.h index 5141c593cc..e045f8c98c 100644 --- a/include/xrpl/protocol/STObject.h +++ b/include/xrpl/protocol/STObject.h @@ -231,6 +231,8 @@ public: getFieldH192(SField const& field) const; uint256 getFieldH256(SField const& field) const; + std::int32_t + getFieldI32(SField const& field) const; AccountID getAccountID(SField const& field) const; @@ -367,6 +369,8 @@ public: void setFieldH256(SField const& field, uint256 const&); void + setFieldI32(SField const& field, std::int32_t); + void setFieldVL(SField const& field, Blob const&); void setFieldVL(SField const& field, Slice const&); diff --git a/include/xrpl/protocol/TER.h b/include/xrpl/protocol/TER.h index 39ba667ead..a5d6701e80 100644 --- a/include/xrpl/protocol/TER.h +++ b/include/xrpl/protocol/TER.h @@ -675,7 +675,8 @@ isTerRetry(TER x) noexcept inline bool isTesSuccess(TER x) noexcept { - return (x == tesSUCCESS); + // Makes use of TERSubset::operator bool() + return !(x); } inline bool diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index ce1df77cbf..125182edd3 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -33,11 +33,11 @@ // in include/xrpl/protocol/Feature.h. XRPL_FEATURE(Sponsor, Supported::yes, VoteBehavior::DefaultNo) -XRPL_FIX (IncludeKeyletFields, Supported::no, VoteBehavior::DefaultNo) +XRPL_FIX (IncludeKeyletFields, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(DynamicMPT, Supported::no, VoteBehavior::DefaultNo) XRPL_FIX (TokenEscrowV1, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (DelegateV1_1, Supported::no, VoteBehavior::DefaultNo) -XRPL_FIX (PriceOracleOrder, Supported::no, VoteBehavior::DefaultNo) +XRPL_FIX (PriceOracleOrder, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (MPTDeliveredAmount, Supported::no, VoteBehavior::DefaultNo) XRPL_FIX (AMMClawbackRounding, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(TokenEscrow, Supported::yes, VoteBehavior::DefaultNo) diff --git a/include/xrpl/protocol/detail/sfields.macro b/include/xrpl/protocol/detail/sfields.macro index 4ac095bd1b..3f800a4a64 100644 --- a/include/xrpl/protocol/detail/sfields.macro +++ b/include/xrpl/protocol/detail/sfields.macro @@ -214,6 +214,12 @@ TYPED_SFIELD(sfAssetsMaximum, NUMBER, 3) TYPED_SFIELD(sfAssetsTotal, NUMBER, 4) TYPED_SFIELD(sfLossUnrealized, NUMBER, 5) +// int32 +// NOTE: Do not use `sfDummyInt32`. It's so far the only use of INT32 +// in this file and has been defined here for test only. +// TODO: Replace `sfDummyInt32` with actually useful field. +TYPED_SFIELD(sfDummyInt32, INT32, 1) // for tests only + // currency amount (common) TYPED_SFIELD(sfAmount, AMOUNT, 1) TYPED_SFIELD(sfBalance, AMOUNT, 2) diff --git a/src/libxrpl/net/HTTPClient.cpp b/src/libxrpl/net/HTTPClient.cpp index 964be32dd8..74b8b61ca6 100644 --- a/src/libxrpl/net/HTTPClient.cpp +++ b/src/libxrpl/net/HTTPClient.cpp @@ -383,7 +383,7 @@ public: static boost::regex reStatus{ "\\`HTTP/1\\S+ (\\d{3}) .*\\'"}; // HTTP/1.1 200 OK static boost::regex reSize{ - "\\`.*\\r\\nContent-Length:\\s+([0-9]+).*\\'"}; + "\\`.*\\r\\nContent-Length:\\s+([0-9]+).*\\'", boost::regex::icase}; static boost::regex reBody{"\\`.*\\r\\n\\r\\n(.*)\\'"}; boost::smatch smMatch; diff --git a/src/libxrpl/protocol/Permissions.cpp b/src/libxrpl/protocol/Permissions.cpp index 6a4b0678e0..c9e32c5056 100644 --- a/src/libxrpl/protocol/Permissions.cpp +++ b/src/libxrpl/protocol/Permissions.cpp @@ -147,6 +147,19 @@ Permission::getGranularTxType(GranularPermissionType const& gpType) const return std::nullopt; } +std::optional> const +Permission::getTxFeature(TxType txType) const +{ + auto const txFeaturesIt = txFeatureMap_.find(txType); + XRPL_ASSERT( + txFeaturesIt != txFeatureMap_.end(), + "ripple::Permissions::getTxFeature : tx exists in txFeatureMap_"); + + if (txFeaturesIt->second == uint256{}) + return std::nullopt; + return txFeaturesIt->second; +} + bool Permission::isDelegatable( std::uint32_t const& permissionValue, @@ -166,16 +179,12 @@ Permission::isDelegatable( if (it == delegatableTx_.end()) return false; - auto const txFeaturesIt = txFeatureMap_.find(txType); - XRPL_ASSERT( - txFeaturesIt != txFeatureMap_.end(), - "ripple::Permissions::isDelegatable : tx exists in txFeatureMap_"); + auto const feature = getTxFeature(txType); // fixDelegateV1_1: Delegation is only allowed if the required amendment // for the transaction is enabled. For transactions that do not require // an amendment, delegation is always allowed. - if (txFeaturesIt->second != uint256{} && - !rules.enabled(txFeaturesIt->second)) + if (feature && !rules.enabled(*feature)) return false; } diff --git a/src/libxrpl/protocol/STInteger.cpp b/src/libxrpl/protocol/STInteger.cpp index 5d6c1802cc..355fa4c113 100644 --- a/src/libxrpl/protocol/STInteger.cpp +++ b/src/libxrpl/protocol/STInteger.cpp @@ -249,4 +249,33 @@ STUInt64::getJson(JsonOptions) const return convertToString(value_, 16); // Convert to base 16 } +//------------------------------------------------------------------------------ + +template <> +STInteger::STInteger(SerialIter& sit, SField const& name) + : STInteger(name, sit.get32()) +{ +} + +template <> +SerializedTypeID +STInt32::getSType() const +{ + return STI_INT32; +} + +template <> +std::string +STInt32::getText() const +{ + return std::to_string(value_); +} + +template <> +Json::Value +STInt32::getJson(JsonOptions) const +{ + return value_; +} + } // namespace ripple diff --git a/src/libxrpl/protocol/STObject.cpp b/src/libxrpl/protocol/STObject.cpp index a5f1737418..8127e7987f 100644 --- a/src/libxrpl/protocol/STObject.cpp +++ b/src/libxrpl/protocol/STObject.cpp @@ -647,6 +647,12 @@ STObject::getFieldH256(SField const& field) const return getFieldByValue(field); } +std::int32_t +STObject::getFieldI32(SField const& field) const +{ + return getFieldByValue(field); +} + AccountID STObject::getAccountID(SField const& field) const { @@ -768,6 +774,12 @@ STObject::setFieldH256(SField const& field, uint256 const& v) setFieldUsingSetValue(field, v); } +void +STObject::setFieldI32(SField const& field, std::int32_t v) +{ + setFieldUsingSetValue(field, v); +} + void STObject::setFieldV256(SField const& field, STVector256 const& v) { diff --git a/src/libxrpl/protocol/STParsedJSON.cpp b/src/libxrpl/protocol/STParsedJSON.cpp index 9fbe5e7f91..f99fec6b87 100644 --- a/src/libxrpl/protocol/STParsedJSON.cpp +++ b/src/libxrpl/protocol/STParsedJSON.cpp @@ -563,30 +563,6 @@ parseLeaf( break; } - case STI_UINT192: { - if (!value.isString()) - { - error = bad_type(json_name, fieldName); - return ret; - } - - uint192 num; - - if (auto const s = value.asString(); !num.parseHex(s)) - { - if (!s.empty()) - { - error = invalid_data(json_name, fieldName); - return ret; - } - - num.zero(); - } - - ret = detail::make_stvar(field, num); - break; - } - case STI_UINT160: { if (!value.isString()) { @@ -611,6 +587,30 @@ parseLeaf( break; } + case STI_UINT192: { + if (!value.isString()) + { + error = bad_type(json_name, fieldName); + return ret; + } + + uint192 num; + + if (auto const s = value.asString(); !num.parseHex(s)) + { + if (!s.empty()) + { + error = invalid_data(json_name, fieldName); + return ret; + } + + num.zero(); + } + + ret = detail::make_stvar(field, num); + break; + } + case STI_UINT256: { if (!value.isString()) { @@ -635,6 +635,52 @@ parseLeaf( break; } + case STI_INT32: + try + { + if (value.isString()) + { + ret = detail::make_stvar( + field, + beast::lexicalCastThrow( + value.asString())); + } + else if (value.isInt()) + { + // future-proofing - a static assert failure if the JSON + // library ever supports larger ints + // In such case, we will need additional bounds checks here + static_assert( + std::is_same_v); + ret = detail::make_stvar(field, value.asInt()); + } + else if (value.isUInt()) + { + auto const uintValue = value.asUInt(); + if (uintValue > + static_cast( + std::numeric_limits::max())) + { + error = out_of_range(json_name, fieldName); + return ret; + } + ret = detail::make_stvar( + field, static_cast(uintValue)); + } + else + { + error = bad_type(json_name, fieldName); + return ret; + } + } + catch (std::exception const&) + { + error = invalid_data(json_name, fieldName); + return ret; + } + + break; + case STI_VL: if (!value.isString()) { @@ -1120,8 +1166,7 @@ parseArray( Json::Value const objectFields(json[i][objectName]); std::stringstream ss; - ss << json_name << "." - << "[" << i << "]." << objectName; + ss << json_name << "." << "[" << i << "]." << objectName; auto ret = parseObject( ss.str(), objectFields, nameField, depth + 1, error); diff --git a/src/libxrpl/protocol/STVar.cpp b/src/libxrpl/protocol/STVar.cpp index 24954c4add..c46effb47e 100644 --- a/src/libxrpl/protocol/STVar.cpp +++ b/src/libxrpl/protocol/STVar.cpp @@ -208,6 +208,9 @@ STVar::constructST(SerializedTypeID id, int depth, Args&&... args) case STI_UINT256: construct(std::forward(args)...); return; + case STI_INT32: + construct(std::forward(args)...); + return; case STI_VECTOR256: construct(std::forward(args)...); return; diff --git a/src/libxrpl/protocol/Serializer.cpp b/src/libxrpl/protocol/Serializer.cpp index b8a68d28b8..098e68d2b2 100644 --- a/src/libxrpl/protocol/Serializer.cpp +++ b/src/libxrpl/protocol/Serializer.cpp @@ -83,6 +83,12 @@ Serializer::addInteger(std::uint64_t i) { return add64(i); } +template <> +int +Serializer::addInteger(std::int32_t i) +{ + return add32(i); +} int Serializer::addRaw(Blob const& vector) diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 6a80b721ce..3e0d65894f 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -3572,7 +3572,7 @@ private: env.current()->rules(), tapNONE, env.journal); - auto pf = AMMBid::preflight(pfctx); + auto pf = Transactor::invokePreflight(pfctx); BEAST_EXPECT(pf == temDISABLED); env.app().config().features.insert(featureAMM); } @@ -3587,7 +3587,7 @@ private: env.current()->rules(), tapNONE, env.journal); - auto pf = AMMBid::preflight(pfctx); + auto pf = Transactor::invokePreflight(pfctx); BEAST_EXPECT(pf != tesSUCCESS); } @@ -3602,7 +3602,7 @@ private: env.current()->rules(), tapNONE, env.journal); - auto pf = AMMBid::preflight(pfctx); + auto pf = Transactor::invokePreflight(pfctx); BEAST_EXPECT(pf == temBAD_AMM_TOKENS); } } diff --git a/src/test/app/FeeVote_test.cpp b/src/test/app/FeeVote_test.cpp index ba3d379219..4fe0a62e3b 100644 --- a/src/test/app/FeeVote_test.cpp +++ b/src/test/app/FeeVote_test.cpp @@ -19,11 +19,203 @@ #include +#include +#include +#include + #include +#include +#include +#include +#include +#include +#include namespace ripple { namespace test { +struct FeeSettingsFields +{ + std::optional baseFee = std::nullopt; + std::optional reserveBase = std::nullopt; + std::optional reserveIncrement = std::nullopt; + std::optional referenceFeeUnits = std::nullopt; + std::optional baseFeeDrops = std::nullopt; + std::optional reserveBaseDrops = std::nullopt; + std::optional reserveIncrementDrops = std::nullopt; +}; + +STTx +createFeeTx( + Rules const& rules, + std::uint32_t seq, + FeeSettingsFields const& fields) +{ + auto fill = [&](auto& obj) { + obj.setAccountID(sfAccount, AccountID()); + obj.setFieldU32(sfLedgerSequence, seq); + + if (rules.enabled(featureXRPFees)) + { + // New XRPFees format - all three fields are REQUIRED + obj.setFieldAmount( + sfBaseFeeDrops, + fields.baseFeeDrops ? *fields.baseFeeDrops : XRPAmount{0}); + obj.setFieldAmount( + sfReserveBaseDrops, + fields.reserveBaseDrops ? *fields.reserveBaseDrops + : XRPAmount{0}); + obj.setFieldAmount( + sfReserveIncrementDrops, + fields.reserveIncrementDrops ? *fields.reserveIncrementDrops + : XRPAmount{0}); + } + else + { + // Legacy format - all four fields are REQUIRED + obj.setFieldU64(sfBaseFee, fields.baseFee ? *fields.baseFee : 0); + obj.setFieldU32( + sfReserveBase, fields.reserveBase ? *fields.reserveBase : 0); + obj.setFieldU32( + sfReserveIncrement, + fields.reserveIncrement ? *fields.reserveIncrement : 0); + obj.setFieldU32( + sfReferenceFeeUnits, + fields.referenceFeeUnits ? *fields.referenceFeeUnits : 0); + } + }; + return STTx(ttFEE, fill); +} + +STTx +createInvalidFeeTx( + Rules const& rules, + std::uint32_t seq, + bool missingRequiredFields = true, + bool wrongFeatureFields = false, + std::uint32_t uniqueValue = 42) +{ + auto fill = [&](auto& obj) { + obj.setAccountID(sfAccount, AccountID()); + obj.setFieldU32(sfLedgerSequence, seq); + + if (wrongFeatureFields) + { + if (rules.enabled(featureXRPFees)) + { + obj.setFieldU64(sfBaseFee, 10 + uniqueValue); + obj.setFieldU32(sfReserveBase, 200000); + obj.setFieldU32(sfReserveIncrement, 50000); + obj.setFieldU32(sfReferenceFeeUnits, 10); + } + else + { + obj.setFieldAmount(sfBaseFeeDrops, XRPAmount{10 + uniqueValue}); + obj.setFieldAmount(sfReserveBaseDrops, XRPAmount{200000}); + obj.setFieldAmount(sfReserveIncrementDrops, XRPAmount{50000}); + } + } + else if (!missingRequiredFields) + { + // Create valid transaction (all required fields present) + if (rules.enabled(featureXRPFees)) + { + obj.setFieldAmount(sfBaseFeeDrops, XRPAmount{10 + uniqueValue}); + obj.setFieldAmount(sfReserveBaseDrops, XRPAmount{200000}); + obj.setFieldAmount(sfReserveIncrementDrops, XRPAmount{50000}); + } + else + { + obj.setFieldU64(sfBaseFee, 10 + uniqueValue); + obj.setFieldU32(sfReserveBase, 200000); + obj.setFieldU32(sfReserveIncrement, 50000); + obj.setFieldU32(sfReferenceFeeUnits, 10); + } + } + // If missingRequiredFields is true, we don't add the required fields + // (default behavior) + }; + return STTx(ttFEE, fill); +} + +bool +applyFeeAndTestResult(jtx::Env& env, OpenView& view, STTx const& tx) +{ + auto const res = + apply(env.app(), view, tx, ApplyFlags::tapNONE, env.journal); + return res.ter == tesSUCCESS; +} + +bool +verifyFeeObject( + std::shared_ptr const& ledger, + Rules const& rules, + FeeSettingsFields const& expected) +{ + auto const feeObject = ledger->read(keylet::fees()); + if (!feeObject) + return false; + + auto checkEquality = [&](auto const& field, auto const& expected) { + if (!feeObject->isFieldPresent(field)) + return false; + return feeObject->at(field) == expected; + }; + + if (rules.enabled(featureXRPFees)) + { + if (feeObject->isFieldPresent(sfBaseFee) || + feeObject->isFieldPresent(sfReserveBase) || + feeObject->isFieldPresent(sfReserveIncrement) || + feeObject->isFieldPresent(sfReferenceFeeUnits)) + return false; + + if (!checkEquality( + sfBaseFeeDrops, expected.baseFeeDrops.value_or(XRPAmount{0}))) + return false; + if (!checkEquality( + sfReserveBaseDrops, + expected.reserveBaseDrops.value_or(XRPAmount{0}))) + return false; + if (!checkEquality( + sfReserveIncrementDrops, + expected.reserveIncrementDrops.value_or(XRPAmount{0}))) + return false; + } + else + { + if (feeObject->isFieldPresent(sfBaseFeeDrops) || + feeObject->isFieldPresent(sfReserveBaseDrops) || + feeObject->isFieldPresent(sfReserveIncrementDrops)) + return false; + + // Read sfBaseFee as a hex string and compare to expected.baseFee + if (!checkEquality(sfBaseFee, expected.baseFee)) + return false; + if (!checkEquality(sfReserveBase, expected.reserveBase)) + return false; + if (!checkEquality(sfReserveIncrement, expected.reserveIncrement)) + return false; + if (!checkEquality(sfReferenceFeeUnits, expected.referenceFeeUnits)) + return false; + } + + return true; +} + +std::vector +getTxs(std::shared_ptr const& txSet) +{ + std::vector txs; + for (auto i = txSet->begin(); i != txSet->end(); ++i) + { + auto const data = i->slice(); + auto serialIter = SerialIter(data); + txs.push_back(STTx(serialIter)); + } + return txs; +}; + class FeeVote_test : public beast::unit_test::suite { void @@ -93,10 +285,517 @@ class FeeVote_test : public beast::unit_test::suite } } + void + testBasic() + { + testcase("Basic SetFee transaction"); + + // Test with XRPFees disabled (legacy format) + { + jtx::Env env(*this, jtx::testable_amendments() - featureXRPFees); + auto ledger = std::make_shared( + create_genesis, + env.app().config(), + std::vector{}, + env.app().getNodeFamily()); + + // Create the next ledger to apply transaction to + ledger = std::make_shared( + *ledger, env.app().timeKeeper().closeTime()); + + // Test successful fee transaction with legacy fields + + FeeSettingsFields fields{ + .baseFee = 10, + .reserveBase = 200000, + .reserveIncrement = 50000, + .referenceFeeUnits = 10}; + auto feeTx = createFeeTx(ledger->rules(), ledger->seq(), fields); + + OpenView accum(ledger.get()); + BEAST_EXPECT(applyFeeAndTestResult(env, accum, feeTx)); + accum.apply(*ledger); + + // Verify fee object was created/updated correctly + BEAST_EXPECT(verifyFeeObject(ledger, ledger->rules(), fields)); + } + + // Test with XRPFees enabled (new format) + { + jtx::Env env(*this, jtx::testable_amendments() | featureXRPFees); + auto ledger = std::make_shared( + create_genesis, + env.app().config(), + std::vector{}, + env.app().getNodeFamily()); + + // Create the next ledger to apply transaction to + ledger = std::make_shared( + *ledger, env.app().timeKeeper().closeTime()); + + FeeSettingsFields fields{ + .baseFeeDrops = XRPAmount{10}, + .reserveBaseDrops = XRPAmount{200000}, + .reserveIncrementDrops = XRPAmount{50000}}; + // Test successful fee transaction with new fields + auto feeTx = createFeeTx(ledger->rules(), ledger->seq(), fields); + + OpenView accum(ledger.get()); + BEAST_EXPECT(applyFeeAndTestResult(env, accum, feeTx)); + accum.apply(*ledger); + + // Verify fee object was created/updated correctly + BEAST_EXPECT(verifyFeeObject(ledger, ledger->rules(), fields)); + } + } + + void + testTransactionValidation() + { + testcase("Fee Transaction Validation"); + + { + jtx::Env env(*this, jtx::testable_amendments() - featureXRPFees); + auto ledger = std::make_shared( + create_genesis, + env.app().config(), + std::vector{}, + env.app().getNodeFamily()); + + // Create the next ledger to apply transaction to + ledger = std::make_shared( + *ledger, env.app().timeKeeper().closeTime()); + + // Test transaction with missing required legacy fields + auto invalidTx = createInvalidFeeTx( + ledger->rules(), ledger->seq(), true, false, 1); + OpenView accum(ledger.get()); + BEAST_EXPECT(!applyFeeAndTestResult(env, accum, invalidTx)); + + // Test transaction with new format fields when XRPFees is disabled + auto disallowedTx = createInvalidFeeTx( + ledger->rules(), ledger->seq(), false, true, 2); + BEAST_EXPECT(!applyFeeAndTestResult(env, accum, disallowedTx)); + } + + { + jtx::Env env(*this, jtx::testable_amendments() | featureXRPFees); + auto ledger = std::make_shared( + create_genesis, + env.app().config(), + std::vector{}, + env.app().getNodeFamily()); + + // Create the next ledger to apply transaction to + ledger = std::make_shared( + *ledger, env.app().timeKeeper().closeTime()); + + // Test transaction with missing required new fields + auto invalidTx = createInvalidFeeTx( + ledger->rules(), ledger->seq(), true, false, 3); + OpenView accum(ledger.get()); + BEAST_EXPECT(!applyFeeAndTestResult(env, accum, invalidTx)); + + // Test transaction with legacy fields when XRPFees is enabled + auto disallowedTx = createInvalidFeeTx( + ledger->rules(), ledger->seq(), false, true, 4); + BEAST_EXPECT(!applyFeeAndTestResult(env, accum, disallowedTx)); + } + } + + void + testPseudoTransactionProperties() + { + testcase("Pseudo Transaction Properties"); + + jtx::Env env(*this, jtx::testable_amendments()); + auto ledger = std::make_shared( + create_genesis, + env.app().config(), + std::vector{}, + env.app().getNodeFamily()); + + // Create the next ledger to apply transaction to + ledger = std::make_shared( + *ledger, env.app().timeKeeper().closeTime()); + + auto feeTx = createFeeTx( + ledger->rules(), + ledger->seq(), + {.baseFeeDrops = XRPAmount{10}, + .reserveBaseDrops = XRPAmount{200000}, + .reserveIncrementDrops = XRPAmount{50000}}); + + // Verify pseudo-transaction properties + BEAST_EXPECT(feeTx.getAccountID(sfAccount) == AccountID()); + BEAST_EXPECT(feeTx.getFieldAmount(sfFee) == XRPAmount{0}); + BEAST_EXPECT(feeTx.getSigningPubKey().empty()); + BEAST_EXPECT(feeTx.getSignature().empty()); + BEAST_EXPECT(!feeTx.isFieldPresent(sfSigners)); + BEAST_EXPECT(feeTx.getFieldU32(sfSequence) == 0); + BEAST_EXPECT(!feeTx.isFieldPresent(sfPreviousTxnID)); + + // But can be applied to a closed ledger + { + OpenView closedAccum(ledger.get()); + BEAST_EXPECT(applyFeeAndTestResult(env, closedAccum, feeTx)); + } + } + + void + testMultipleFeeUpdates() + { + testcase("Multiple Fee Updates"); + + jtx::Env env(*this, jtx::testable_amendments() | featureXRPFees); + auto ledger = std::make_shared( + create_genesis, + env.app().config(), + std::vector{}, + env.app().getNodeFamily()); + + ledger = std::make_shared( + *ledger, env.app().timeKeeper().closeTime()); + + FeeSettingsFields fields1{ + .baseFeeDrops = XRPAmount{10}, + .reserveBaseDrops = XRPAmount{200000}, + .reserveIncrementDrops = XRPAmount{50000}}; + auto feeTx1 = createFeeTx(ledger->rules(), ledger->seq(), fields1); + + { + OpenView accum(ledger.get()); + BEAST_EXPECT(applyFeeAndTestResult(env, accum, feeTx1)); + accum.apply(*ledger); + } + + BEAST_EXPECT(verifyFeeObject(ledger, ledger->rules(), fields1)); + + // Apply second fee transaction with different values + ledger = std::make_shared( + *ledger, env.app().timeKeeper().closeTime()); + + FeeSettingsFields fields2{ + .baseFeeDrops = XRPAmount{20}, + .reserveBaseDrops = XRPAmount{300000}, + .reserveIncrementDrops = XRPAmount{75000}}; + auto feeTx2 = createFeeTx(ledger->rules(), ledger->seq(), fields2); + + { + OpenView accum(ledger.get()); + BEAST_EXPECT(applyFeeAndTestResult(env, accum, feeTx2)); + accum.apply(*ledger); + } + + // Verify second update overwrote the first + BEAST_EXPECT(verifyFeeObject(ledger, ledger->rules(), fields2)); + } + + void + testWrongLedgerSequence() + { + testcase("Wrong Ledger Sequence"); + + jtx::Env env(*this, jtx::testable_amendments() | featureXRPFees); + auto ledger = std::make_shared( + create_genesis, + env.app().config(), + std::vector{}, + env.app().getNodeFamily()); + + ledger = std::make_shared( + *ledger, env.app().timeKeeper().closeTime()); + + // Test transaction with wrong ledger sequence + auto feeTx = createFeeTx( + ledger->rules(), + ledger->seq() + 5, // Wrong sequence (should be ledger->seq()) + {.baseFeeDrops = XRPAmount{10}, + .reserveBaseDrops = XRPAmount{200000}, + .reserveIncrementDrops = XRPAmount{50000}}); + + OpenView accum(ledger.get()); + + // The transaction should still succeed as long as other fields are + // valid + // The ledger sequence field is only used for informational purposes + BEAST_EXPECT(applyFeeAndTestResult(env, accum, feeTx)); + } + + void + testPartialFieldUpdates() + { + testcase("Partial Field Updates"); + + jtx::Env env(*this, jtx::testable_amendments() | featureXRPFees); + auto ledger = std::make_shared( + create_genesis, + env.app().config(), + std::vector{}, + env.app().getNodeFamily()); + + ledger = std::make_shared( + *ledger, env.app().timeKeeper().closeTime()); + + FeeSettingsFields fields1{ + .baseFeeDrops = XRPAmount{10}, + .reserveBaseDrops = XRPAmount{200000}, + .reserveIncrementDrops = XRPAmount{50000}}; + auto feeTx1 = createFeeTx(ledger->rules(), ledger->seq(), fields1); + + { + OpenView accum(ledger.get()); + BEAST_EXPECT(applyFeeAndTestResult(env, accum, feeTx1)); + accum.apply(*ledger); + } + + BEAST_EXPECT(verifyFeeObject(ledger, ledger->rules(), fields1)); + + ledger = std::make_shared( + *ledger, env.app().timeKeeper().closeTime()); + + // Apply partial update (only some fields) + FeeSettingsFields fields2{ + .baseFeeDrops = XRPAmount{20}, + .reserveBaseDrops = XRPAmount{200000}}; + auto feeTx2 = createFeeTx(ledger->rules(), ledger->seq(), fields2); + + { + OpenView accum(ledger.get()); + BEAST_EXPECT(applyFeeAndTestResult(env, accum, feeTx2)); + accum.apply(*ledger); + } + + // Verify the partial update worked + BEAST_EXPECT(verifyFeeObject(ledger, ledger->rules(), fields2)); + } + + void + testSingleInvalidTransaction() + { + testcase("Single Invalid Transaction"); + + jtx::Env env(*this, jtx::testable_amendments() | featureXRPFees); + auto ledger = std::make_shared( + create_genesis, + env.app().config(), + std::vector{}, + env.app().getNodeFamily()); + + ledger = std::make_shared( + *ledger, env.app().timeKeeper().closeTime()); + + // Test invalid transaction with non-zero account - this should fail + // validation + auto invalidTx = STTx(ttFEE, [&](auto& obj) { + obj.setAccountID( + sfAccount, + AccountID(1)); // Should be zero (this makes it invalid) + obj.setFieldU32(sfLedgerSequence, ledger->seq()); + obj.setFieldAmount(sfBaseFeeDrops, XRPAmount{10}); + obj.setFieldAmount(sfReserveBaseDrops, XRPAmount{200000}); + obj.setFieldAmount(sfReserveIncrementDrops, XRPAmount{50000}); + }); + + OpenView accum(ledger.get()); + BEAST_EXPECT(!applyFeeAndTestResult(env, accum, invalidTx)); + } + + void + testDoValidation() + { + testcase("doValidation"); + + using namespace jtx; + + FeeSetup setup; + setup.reference_fee = 42; + setup.account_reserve = 1234567; + setup.owner_reserve = 7654321; + + // Test with XRPFees enabled + { + Env env(*this, testable_amendments() | featureXRPFees); + auto feeVote = make_FeeVote(setup, env.app().journal("FeeVote")); + + auto ledger = std::make_shared( + create_genesis, + env.app().config(), + std::vector{}, + env.app().getNodeFamily()); + + auto sec = randomSecretKey(); + auto pub = derivePublicKey(KeyType::secp256k1, sec); + + auto val = std::make_shared( + env.app().timeKeeper().now(), + pub, + sec, + calcNodeID(pub), + [](STValidation& v) { + v.setFieldU32(sfLedgerSequence, 12345); + }); + + // Use the current ledger's fees as the "current" fees for + // doValidation + auto const& currentFees = ledger->fees(); + + feeVote->doValidation(currentFees, ledger->rules(), *val); + + BEAST_EXPECT(val->isFieldPresent(sfBaseFeeDrops)); + BEAST_EXPECT( + val->getFieldAmount(sfBaseFeeDrops) == + XRPAmount(setup.reference_fee)); + } + + // Test with XRPFees disabled (legacy format) + { + Env env(*this, testable_amendments() - featureXRPFees); + auto feeVote = make_FeeVote(setup, env.app().journal("FeeVote")); + + auto ledger = std::make_shared( + create_genesis, + env.app().config(), + std::vector{}, + env.app().getNodeFamily()); + + auto sec = randomSecretKey(); + auto pub = derivePublicKey(KeyType::secp256k1, sec); + + auto val = std::make_shared( + env.app().timeKeeper().now(), + pub, + sec, + calcNodeID(pub), + [](STValidation& v) { + v.setFieldU32(sfLedgerSequence, 12345); + }); + + auto const& currentFees = ledger->fees(); + + feeVote->doValidation(currentFees, ledger->rules(), *val); + + // In legacy mode, should vote using legacy fields + BEAST_EXPECT(val->isFieldPresent(sfBaseFee)); + BEAST_EXPECT(val->getFieldU64(sfBaseFee) == setup.reference_fee); + } + } + + void + testDoVoting() + { + testcase("doVoting"); + + using namespace jtx; + + FeeSetup setup; + setup.reference_fee = 42; + setup.account_reserve = 1234567; + setup.owner_reserve = 7654321; + + Env env(*this, testable_amendments() | featureXRPFees); + + // establish what the current fees are + BEAST_EXPECT( + env.current()->fees().base == XRPAmount{UNIT_TEST_REFERENCE_FEE}); + BEAST_EXPECT(env.current()->fees().reserve == XRPAmount{200'000'000}); + BEAST_EXPECT(env.current()->fees().increment == XRPAmount{50'000'000}); + + auto feeVote = make_FeeVote(setup, env.app().journal("FeeVote")); + auto ledger = std::make_shared( + create_genesis, + env.app().config(), + std::vector{}, + env.app().getNodeFamily()); + + // doVoting requires a flag ledger (every 256th ledger) + // We need to create a ledger at sequence 256 to make it a flag ledger + for (int i = 0; i < 256 - 1; ++i) + { + ledger = std::make_shared( + *ledger, env.app().timeKeeper().closeTime()); + } + BEAST_EXPECT(ledger->isFlagLedger()); + + // Create some mock validations with fee votes + std::vector> validations; + + for (int i = 0; i < 5; i++) + { + auto sec = randomSecretKey(); + auto pub = derivePublicKey(KeyType::secp256k1, sec); + + auto val = std::make_shared( + env.app().timeKeeper().now(), + pub, + sec, + calcNodeID(pub), + [&](STValidation& v) { + v.setFieldU32(sfLedgerSequence, ledger->seq()); + // Vote for different fees than current + v.setFieldAmount( + sfBaseFeeDrops, XRPAmount{setup.reference_fee}); + v.setFieldAmount( + sfReserveBaseDrops, XRPAmount{setup.account_reserve}); + v.setFieldAmount( + sfReserveIncrementDrops, + XRPAmount{setup.owner_reserve}); + }); + if (i % 2) + val->setTrusted(); + validations.push_back(val); + } + + auto txSet = std::make_shared( + SHAMapType::TRANSACTION, env.app().getNodeFamily()); + + // This should not throw since we have a flag ledger + feeVote->doVoting(ledger, validations, txSet); + + auto const txs = getTxs(txSet); + BEAST_EXPECT(txs.size() == 1); + auto const& feeTx = txs[0]; + + BEAST_EXPECT(feeTx.getTxnType() == ttFEE); + + BEAST_EXPECT(feeTx.getAccountID(sfAccount) == AccountID()); + BEAST_EXPECT(feeTx.getFieldU32(sfLedgerSequence) == ledger->seq() + 1); + + BEAST_EXPECT(feeTx.isFieldPresent(sfBaseFeeDrops)); + BEAST_EXPECT(feeTx.isFieldPresent(sfReserveBaseDrops)); + BEAST_EXPECT(feeTx.isFieldPresent(sfReserveIncrementDrops)); + + // The legacy fields should NOT be present + BEAST_EXPECT(!feeTx.isFieldPresent(sfBaseFee)); + BEAST_EXPECT(!feeTx.isFieldPresent(sfReserveBase)); + BEAST_EXPECT(!feeTx.isFieldPresent(sfReserveIncrement)); + BEAST_EXPECT(!feeTx.isFieldPresent(sfReferenceFeeUnits)); + + // Check the values + BEAST_EXPECT( + feeTx.getFieldAmount(sfBaseFeeDrops) == + XRPAmount{setup.reference_fee}); + BEAST_EXPECT( + feeTx.getFieldAmount(sfReserveBaseDrops) == + XRPAmount{setup.account_reserve}); + BEAST_EXPECT( + feeTx.getFieldAmount(sfReserveIncrementDrops) == + XRPAmount{setup.owner_reserve}); + } + void run() override { testSetup(); + testBasic(); + testTransactionValidation(); + testPseudoTransactionProperties(); + testMultipleFeeUpdates(); + testWrongLedgerSequence(); + testPartialFieldUpdates(); + testSingleInvalidTransaction(); + testDoValidation(); + testDoVoting(); } }; diff --git a/src/test/protocol/STAccount_test.cpp b/src/test/protocol/STAccount_test.cpp index 9476a47c5e..cc318b4458 100644 --- a/src/test/protocol/STAccount_test.cpp +++ b/src/test/protocol/STAccount_test.cpp @@ -122,10 +122,27 @@ struct STAccount_test : public beast::unit_test::suite } } + void + testAccountID() + { + auto const s = "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh"; + if (auto const parsed = parseBase58(s); BEAST_EXPECT(parsed)) + { + BEAST_EXPECT(toBase58(*parsed) == s); + } + + { + auto const s = + "âabcd1rNxp4h8apvRis6mJf9Sh8C6iRxfrDWNâabcdAVâ\xc2\x80\xc2\x8f"; + BEAST_EXPECT(!parseBase58(s)); + } + } + void run() override { testSTAccount(); + testAccountID(); } }; diff --git a/src/test/protocol/STInteger_test.cpp b/src/test/protocol/STInteger_test.cpp index f4572e49bd..6c4cdd6fcf 100644 --- a/src/test/protocol/STInteger_test.cpp +++ b/src/test/protocol/STInteger_test.cpp @@ -30,6 +30,7 @@ struct STInteger_test : public beast::unit_test::suite void testUInt8() { + testcase("UInt8"); STUInt8 u8(255); BEAST_EXPECT(u8.value() == 255); BEAST_EXPECT(u8.getText() == "255"); @@ -56,6 +57,7 @@ struct STInteger_test : public beast::unit_test::suite void testUInt16() { + testcase("UInt16"); STUInt16 u16(65535); BEAST_EXPECT(u16.value() == 65535); BEAST_EXPECT(u16.getText() == "65535"); @@ -80,6 +82,7 @@ struct STInteger_test : public beast::unit_test::suite void testUInt32() { + testcase("UInt32"); STUInt32 u32(4'294'967'295u); BEAST_EXPECT(u32.value() == 4'294'967'295u); BEAST_EXPECT(u32.getText() == "4294967295"); @@ -102,6 +105,7 @@ struct STInteger_test : public beast::unit_test::suite void testUInt64() { + testcase("UInt64"); STUInt64 u64(0xFFFFFFFFFFFFFFFFull); BEAST_EXPECT(u64.value() == 0xFFFFFFFFFFFFFFFFull); BEAST_EXPECT(u64.getText() == "18446744073709551615"); @@ -120,6 +124,29 @@ struct STInteger_test : public beast::unit_test::suite u64_2.getJson(JsonOptions::none) == "18446744073709551615"); } + void + testInt32() + { + testcase("Int32"); + { + int const minInt32 = -2147483648; + STInt32 i32(minInt32); + BEAST_EXPECT(i32.value() == minInt32); + BEAST_EXPECT(i32.getText() == "-2147483648"); + BEAST_EXPECT(i32.getSType() == STI_INT32); + BEAST_EXPECT(i32.getJson(JsonOptions::none) == minInt32); + } + + { + int const maxInt32 = 2147483647; + STInt32 i32(maxInt32); + BEAST_EXPECT(i32.value() == maxInt32); + BEAST_EXPECT(i32.getText() == "2147483647"); + BEAST_EXPECT(i32.getSType() == STI_INT32); + BEAST_EXPECT(i32.getJson(JsonOptions::none) == maxInt32); + } + } + void run() override { @@ -127,6 +154,7 @@ struct STInteger_test : public beast::unit_test::suite testUInt16(); testUInt32(); testUInt64(); + testInt32(); } }; diff --git a/src/test/protocol/STParsedJSON_test.cpp b/src/test/protocol/STParsedJSON_test.cpp index 9ecb4c0365..1e1e1fb9f4 100644 --- a/src/test/protocol/STParsedJSON_test.cpp +++ b/src/test/protocol/STParsedJSON_test.cpp @@ -736,6 +736,107 @@ class STParsedJSON_test : public beast::unit_test::suite } } + void + testInt32() + { + testcase("Int32"); + { + Json::Value j; + int const minInt32 = -2147483648; + j[sfDummyInt32] = minInt32; + STParsedJSONObject obj("Test", j); + BEAST_EXPECT(obj.object.has_value()); + if (BEAST_EXPECT(obj.object->isFieldPresent(sfDummyInt32))) + BEAST_EXPECT(obj.object->getFieldI32(sfDummyInt32) == minInt32); + } + + // max value + { + Json::Value j; + int const maxInt32 = 2147483647; + j[sfDummyInt32] = maxInt32; + STParsedJSONObject obj("Test", j); + BEAST_EXPECT(obj.object.has_value()); + if (BEAST_EXPECT(obj.object->isFieldPresent(sfDummyInt32))) + BEAST_EXPECT(obj.object->getFieldI32(sfDummyInt32) == maxInt32); + } + + // max uint value + { + Json::Value j; + unsigned int const maxUInt32 = 2147483647u; + j[sfDummyInt32] = maxUInt32; + STParsedJSONObject obj("Test", j); + BEAST_EXPECT(obj.object.has_value()); + if (BEAST_EXPECT(obj.object->isFieldPresent(sfDummyInt32))) + BEAST_EXPECT( + obj.object->getFieldI32(sfDummyInt32) == + static_cast(maxUInt32)); + } + + // Test with string value + { + Json::Value j; + j[sfDummyInt32] = "2147483647"; + STParsedJSONObject obj("Test", j); + BEAST_EXPECT(obj.object.has_value()); + if (BEAST_EXPECT(obj.object->isFieldPresent(sfDummyInt32))) + BEAST_EXPECT( + obj.object->getFieldI32(sfDummyInt32) == 2147483647u); + } + + // Test with string negative value + { + Json::Value j; + int value = -2147483648; + j[sfDummyInt32] = std::to_string(value); + STParsedJSONObject obj("Test", j); + BEAST_EXPECT(obj.object.has_value()); + if (BEAST_EXPECT(obj.object->isFieldPresent(sfDummyInt32))) + BEAST_EXPECT(obj.object->getFieldI32(sfDummyInt32) == value); + } + + // Test out of range value for int32 (negative) + { + Json::Value j; + j[sfDummyInt32] = "-2147483649"; + STParsedJSONObject obj("Test", j); + BEAST_EXPECT(!obj.object.has_value()); + } + + // Test out of range value for int32 (positive) + { + Json::Value j; + j[sfDummyInt32] = 2147483648u; + STParsedJSONObject obj("Test", j); + BEAST_EXPECT(!obj.object.has_value()); + } + + // Test string value out of range + { + Json::Value j; + j[sfDummyInt32] = "2147483648"; + STParsedJSONObject obj("Test", j); + BEAST_EXPECT(!obj.object.has_value()); + } + + // Test bad_type (arrayValue) + { + Json::Value j; + j[sfDummyInt32] = Json::Value(Json::arrayValue); + STParsedJSONObject obj("Test", j); + BEAST_EXPECT(!obj.object.has_value()); + } + + // Test bad_type (objectValue) + { + Json::Value j; + j[sfDummyInt32] = Json::Value(Json::objectValue); + STParsedJSONObject obj("Test", j); + BEAST_EXPECT(!obj.object.has_value()); + } + } + void testBlob() { @@ -1338,8 +1439,7 @@ class STParsedJSON_test : public beast::unit_test::suite issueJson["issuer"] = "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh"; j[sfAsset] = issueJson; STParsedJSONObject obj("Test", j); - if (BEAST_EXPECTS( - obj.object.has_value(), obj.error.toStyledString())) + if (BEAST_EXPECT(obj.object.has_value())) { BEAST_EXPECT(obj.object->isFieldPresent(sfAsset)); auto const& issueField = (*obj.object)[sfAsset]; @@ -2235,6 +2335,7 @@ class STParsedJSON_test : public beast::unit_test::suite testUInt160(); testUInt192(); testUInt256(); + testInt32(); testBlob(); testVector256(); testAccount(); diff --git a/src/test/rpc/AccountSet_test.cpp b/src/test/rpc/AccountSet_test.cpp index 2383434b9d..3826ecbcb4 100644 --- a/src/test/rpc/AccountSet_test.cpp +++ b/src/test/rpc/AccountSet_test.cpp @@ -19,6 +19,8 @@ #include +#include + #include #include #include @@ -579,6 +581,32 @@ public: env.close(); } + void + testBadSigningKey() + { + using namespace test::jtx; + testcase("Bad signing key"); + Env env(*this); + Account const alice("alice"); + + env.fund(XRP(10000), alice); + env.close(); + + auto jtx = env.jt(noop("alice"), ter(temBAD_SIGNATURE)); + if (!BEAST_EXPECT(jtx.stx)) + return; + auto stx = std::make_shared(*jtx.stx); + stx->at(sfSigningPubKey) = makeSlice(std::string("badkey")); + + env.app().openLedger().modify([&](OpenView& view, beast::Journal j) { + auto const result = + ripple::apply(env.app(), view, *stx, tapNONE, j); + BEAST_EXPECT(result.ter == temBAD_SIGNATURE); + BEAST_EXPECT(!result.applied); + return result.applied; + }); + } + void run() override { @@ -595,6 +623,7 @@ public: testRequireAuthWithDir(); testTransferRate(); testTicket(); + testBadSigningKey(); } }; diff --git a/src/tests/libxrpl/CMakeLists.txt b/src/tests/libxrpl/CMakeLists.txt index 68c6fa6cb3..f97283c955 100644 --- a/src/tests/libxrpl/CMakeLists.txt +++ b/src/tests/libxrpl/CMakeLists.txt @@ -12,3 +12,5 @@ xrpl_add_test(basics) target_link_libraries(xrpl.test.basics PRIVATE xrpl.imports.test) xrpl_add_test(crypto) target_link_libraries(xrpl.test.crypto PRIVATE xrpl.imports.test) +xrpl_add_test(net) +target_link_libraries(xrpl.test.net PRIVATE xrpl.imports.test) diff --git a/src/tests/libxrpl/net/HTTPClient.cpp b/src/tests/libxrpl/net/HTTPClient.cpp new file mode 100644 index 0000000000..4d50c47220 --- /dev/null +++ b/src/tests/libxrpl/net/HTTPClient.cpp @@ -0,0 +1,346 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 Ripple Labs Inc. + + 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 +#include +#include + +using namespace ripple; + +namespace { + +// Simple HTTP server using Beast for testing +class TestHTTPServer +{ +private: + boost::asio::io_context ioc_; + boost::asio::ip::tcp::acceptor acceptor_; + boost::asio::ip::tcp::endpoint endpoint_; + std::atomic running_{true}; + unsigned short port_; + + // Custom headers to return + std::map custom_headers_; + std::string response_body_; + unsigned int status_code_{200}; + +public: + TestHTTPServer() : acceptor_(ioc_), port_(0) + { + // Bind to any available port + endpoint_ = {boost::asio::ip::tcp::v4(), 0}; + acceptor_.open(endpoint_.protocol()); + acceptor_.set_option(boost::asio::socket_base::reuse_address(true)); + acceptor_.bind(endpoint_); + acceptor_.listen(); + + // Get the actual port that was assigned + port_ = acceptor_.local_endpoint().port(); + + accept(); + } + + ~TestHTTPServer() + { + stop(); + } + + boost::asio::io_context& + ioc() + { + return ioc_; + } + + unsigned short + port() const + { + return port_; + } + + void + setHeader(std::string const& name, std::string const& value) + { + custom_headers_[name] = value; + } + + void + setResponseBody(std::string const& body) + { + response_body_ = body; + } + + void + setStatusCode(unsigned int code) + { + status_code_ = code; + } + +private: + void + stop() + { + running_ = false; + acceptor_.close(); + } + + void + accept() + { + if (!running_) + return; + + acceptor_.async_accept( + ioc_, + endpoint_, + [&](boost::system::error_code const& error, + boost::asio::ip::tcp::socket peer) { + if (!running_) + return; + + if (!error) + { + handleConnection(std::move(peer)); + } + }); + } + + void + handleConnection(boost::asio::ip::tcp::socket socket) + { + try + { + // Read the HTTP request + boost::beast::flat_buffer buffer; + boost::beast::http::request req; + boost::beast::http::read(socket, buffer, req); + + // Create response + boost::beast::http::response res; + res.version(req.version()); + res.result(status_code_); + res.set(boost::beast::http::field::server, "TestServer"); + + // Add custom headers + for (auto const& [name, value] : custom_headers_) + { + res.set(name, value); + } + + // Set body and prepare payload first + res.body() = response_body_; + res.prepare_payload(); + + // Override Content-Length with custom headers after prepare_payload + // This allows us to test case-insensitive header parsing + for (auto const& [name, value] : custom_headers_) + { + if (boost::iequals(name, "Content-Length")) + { + res.erase(boost::beast::http::field::content_length); + res.set(name, value); + } + } + + // Send response + boost::beast::http::write(socket, res); + + // Shutdown socket gracefully + boost::system::error_code ec; + socket.shutdown(boost::asio::ip::tcp::socket::shutdown_send, ec); + } + catch (std::exception const&) + { + // Connection handling errors are expected + } + + if (running_) + accept(); + } +}; + +// Helper function to run HTTP client test +bool +runHTTPTest( + TestHTTPServer& server, + std::string const& path, + std::atomic& completed, + std::atomic& result_status, + std::string& result_data, + boost::system::error_code& result_error) +{ + // Create a null journal for testing + beast::Journal j{beast::Journal::getNullSink()}; + + // Initialize HTTPClient SSL context + HTTPClient::initializeSSLContext("", "", false, j); + + HTTPClient::get( + false, // no SSL + server.ioc(), + "127.0.0.1", + server.port(), + path, + 1024, // max response size + std::chrono::seconds(5), + [&](boost::system::error_code const& ec, + int status, + std::string const& data) -> bool { + result_error = ec; + result_status = status; + result_data = data; + completed = true; + return false; // don't retry + }, + j); + + // Run the IO context until completion + auto start = std::chrono::steady_clock::now(); + while (!completed && + std::chrono::steady_clock::now() - start < std::chrono::seconds(10)) + { + if (server.ioc().run_one() == 0) + { + break; + } + } + + return completed; +} + +} // anonymous namespace + +TEST_CASE("HTTPClient case insensitive Content-Length") +{ + // Test different cases of Content-Length header + std::vector header_cases = { + "Content-Length", // Standard case + "content-length", // Lowercase - this tests the regex icase fix + "CONTENT-LENGTH", // Uppercase + "Content-length", // Mixed case + "content-Length" // Mixed case 2 + }; + + for (auto const& header_name : header_cases) + { + TestHTTPServer server; + std::string test_body = "Hello World!"; + server.setResponseBody(test_body); + server.setHeader(header_name, std::to_string(test_body.size())); + + std::atomic completed{false}; + std::atomic result_status{0}; + std::string result_data; + boost::system::error_code result_error; + + bool test_completed = runHTTPTest( + server, + "/test", + completed, + result_status, + result_data, + result_error); + + // Verify results + CHECK(test_completed); + CHECK(!result_error); + CHECK(result_status == 200); + CHECK(result_data == test_body); + } +} + +TEST_CASE("HTTPClient basic HTTP request") +{ + TestHTTPServer server; + std::string test_body = "Test response body"; + server.setResponseBody(test_body); + server.setHeader("Content-Type", "text/plain"); + + std::atomic completed{false}; + std::atomic result_status{0}; + std::string result_data; + boost::system::error_code result_error; + + bool test_completed = runHTTPTest( + server, "/basic", completed, result_status, result_data, result_error); + + CHECK(test_completed); + CHECK(!result_error); + CHECK(result_status == 200); + CHECK(result_data == test_body); +} + +TEST_CASE("HTTPClient empty response") +{ + TestHTTPServer server; + server.setResponseBody(""); // Empty body + server.setHeader("Content-Length", "0"); + + std::atomic completed{false}; + std::atomic result_status{0}; + std::string result_data; + boost::system::error_code result_error; + + bool test_completed = runHTTPTest( + server, "/empty", completed, result_status, result_data, result_error); + + CHECK(test_completed); + CHECK(!result_error); + CHECK(result_status == 200); + CHECK(result_data.empty()); +} + +TEST_CASE("HTTPClient different status codes") +{ + std::vector status_codes = {200, 404, 500}; + + for (auto status : status_codes) + { + TestHTTPServer server; + server.setStatusCode(status); + server.setResponseBody("Status " + std::to_string(status)); + + std::atomic completed{false}; + std::atomic result_status{0}; + std::string result_data; + boost::system::error_code result_error; + + bool test_completed = runHTTPTest( + server, + "/status", + completed, + result_status, + result_data, + result_error); + + CHECK(test_completed); + CHECK(!result_error); + CHECK(result_status == static_cast(status)); + } +} diff --git a/src/test/protocol/types_test.cpp b/src/tests/libxrpl/net/main.cpp similarity index 56% rename from src/test/protocol/types_test.cpp rename to src/tests/libxrpl/net/main.cpp index 8257d9c649..be9fc14bbf 100644 --- a/src/test/protocol/types_test.cpp +++ b/src/tests/libxrpl/net/main.cpp @@ -1,7 +1,7 @@ //------------------------------------------------------------------------------ /* This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2012, 2013 Ripple Labs Inc. + Copyright (c) 2024 Ripple Labs Inc. Permission to use, copy, modify, and/or distribute this software for any purpose with or without fee is hereby granted, provided that the above @@ -17,36 +17,5 @@ */ //============================================================================== -#include -#include - -namespace ripple { - -struct types_test : public beast::unit_test::suite -{ - void - testAccountID() - { - auto const s = "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh"; - if (auto const parsed = parseBase58(s); BEAST_EXPECT(parsed)) - { - BEAST_EXPECT(toBase58(*parsed) == s); - } - - { - auto const s = - "âabcd1rNxp4h8apvRis6mJf9Sh8C6iRxfrDWNâabcdAVâ\xc2\x80\xc2\x8f"; - BEAST_EXPECT(!parseBase58(s)); - } - } - - void - run() override - { - testAccountID(); - } -}; - -BEAST_DEFINE_TESTSUITE(types, protocol, ripple); - -} // namespace ripple +#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN +#include diff --git a/src/xrpld/app/tx/detail/AMMBid.cpp b/src/xrpld/app/tx/detail/AMMBid.cpp index 806c075c4f..769668b07b 100644 --- a/src/xrpld/app/tx/detail/AMMBid.cpp +++ b/src/xrpld/app/tx/detail/AMMBid.cpp @@ -30,21 +30,15 @@ namespace ripple { +bool +AMMBid::checkExtraFeatures(PreflightContext const& ctx) +{ + return ammEnabled(ctx.rules); +} + NotTEC AMMBid::preflight(PreflightContext const& ctx) { - if (!ammEnabled(ctx.rules)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - { - JLOG(ctx.j.debug()) << "AMM Bid: invalid flags."; - return temINVALID_FLAG; - } - if (auto const res = invalidAMMAssetPair( ctx.tx[sfAsset].get(), ctx.tx[sfAsset2].get())) { @@ -95,7 +89,7 @@ AMMBid::preflight(PreflightContext const& ctx) } } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/AMMBid.h b/src/xrpld/app/tx/detail/AMMBid.h index 4bb3a2adfd..4a527b6a93 100644 --- a/src/xrpld/app/tx/detail/AMMBid.h +++ b/src/xrpld/app/tx/detail/AMMBid.h @@ -71,6 +71,9 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/AMMClawback.cpp b/src/xrpld/app/tx/detail/AMMClawback.cpp index 77d435afae..eb2c1d7816 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.cpp +++ b/src/xrpld/app/tx/detail/AMMClawback.cpp @@ -33,19 +33,15 @@ namespace ripple { +std::uint32_t +AMMClawback::getFlagsMask(PreflightContext const& ctx) +{ + return tfAMMClawbackMask; +} + NotTEC AMMClawback::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureAMMClawback)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; // LCOV_EXCL_LINE - - auto const flags = ctx.tx.getFlags(); - if (flags & tfAMMClawbackMask) - return temINVALID_FLAG; - AccountID const issuer = ctx.tx[sfAccount]; AccountID const holder = ctx.tx[sfHolder]; @@ -63,6 +59,8 @@ AMMClawback::preflight(PreflightContext const& ctx) if (isXRP(asset)) return temMALFORMED; + auto const flags = ctx.tx.getFlags(); + if (flags & tfClawTwoAssets && asset.account != asset2.account) { JLOG(ctx.j.trace()) @@ -88,7 +86,7 @@ AMMClawback::preflight(PreflightContext const& ctx) if (clawAmount && *clawAmount <= beast::zero) return temBAD_AMOUNT; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/AMMClawback.h b/src/xrpld/app/tx/detail/AMMClawback.h index fdcfc53e2c..1984937971 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.h +++ b/src/xrpld/app/tx/detail/AMMClawback.h @@ -33,6 +33,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/AMMCreate.cpp b/src/xrpld/app/tx/detail/AMMCreate.cpp index e922530f5f..9a4361ca49 100644 --- a/src/xrpld/app/tx/detail/AMMCreate.cpp +++ b/src/xrpld/app/tx/detail/AMMCreate.cpp @@ -31,21 +31,15 @@ namespace ripple { +bool +AMMCreate::checkExtraFeatures(PreflightContext const& ctx) +{ + return ammEnabled(ctx.rules); +} + NotTEC AMMCreate::preflight(PreflightContext const& ctx) { - if (!ammEnabled(ctx.rules)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - { - JLOG(ctx.j.debug()) << "AMM Instance: invalid flags."; - return temINVALID_FLAG; - } - auto const amount = ctx.tx[sfAmount]; auto const amount2 = ctx.tx[sfAmount2]; @@ -74,14 +68,14 @@ AMMCreate::preflight(PreflightContext const& ctx) return temBAD_FEE; } - return preflight2(ctx); + return tesSUCCESS; } XRPAmount AMMCreate::calculateBaseFee(ReadView const& view, STTx const& tx) { // The fee required for AMMCreate is one owner reserve. - return view.fees().increment; + return calculateOwnerReserveFee(view, tx); } TER diff --git a/src/xrpld/app/tx/detail/AMMCreate.h b/src/xrpld/app/tx/detail/AMMCreate.h index 189d66a55a..98231e5554 100644 --- a/src/xrpld/app/tx/detail/AMMCreate.h +++ b/src/xrpld/app/tx/detail/AMMCreate.h @@ -63,6 +63,9 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/AMMDelete.cpp b/src/xrpld/app/tx/detail/AMMDelete.cpp index 004e0b2229..663a4c4b0a 100644 --- a/src/xrpld/app/tx/detail/AMMDelete.cpp +++ b/src/xrpld/app/tx/detail/AMMDelete.cpp @@ -27,22 +27,16 @@ namespace ripple { +bool +AMMDelete::checkExtraFeatures(PreflightContext const& ctx) +{ + return ammEnabled(ctx.rules); +} + NotTEC AMMDelete::preflight(PreflightContext const& ctx) { - if (!ammEnabled(ctx.rules)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - { - JLOG(ctx.j.debug()) << "AMM Delete: invalid flags."; - return temINVALID_FLAG; - } - - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/AMMDelete.h b/src/xrpld/app/tx/detail/AMMDelete.h index 19885b1dad..36dace2e18 100644 --- a/src/xrpld/app/tx/detail/AMMDelete.h +++ b/src/xrpld/app/tx/detail/AMMDelete.h @@ -39,6 +39,9 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/AMMDeposit.cpp b/src/xrpld/app/tx/detail/AMMDeposit.cpp index 5701bab094..ba8a0bf016 100644 --- a/src/xrpld/app/tx/detail/AMMDeposit.cpp +++ b/src/xrpld/app/tx/detail/AMMDeposit.cpp @@ -29,21 +29,23 @@ namespace ripple { +bool +AMMDeposit::checkExtraFeatures(PreflightContext const& ctx) +{ + return ammEnabled(ctx.rules); +} + +std::uint32_t +AMMDeposit::getFlagsMask(PreflightContext const& ctx) + +{ + return tfDepositMask; +} + NotTEC AMMDeposit::preflight(PreflightContext const& ctx) { - if (!ammEnabled(ctx.rules)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - auto const flags = ctx.tx.getFlags(); - if (flags & tfDepositMask) - { - JLOG(ctx.j.debug()) << "AMM Deposit: invalid flags."; - return temINVALID_FLAG; - } auto const amount = ctx.tx[~sfAmount]; auto const amount2 = ctx.tx[~sfAmount2]; @@ -159,7 +161,7 @@ AMMDeposit::preflight(PreflightContext const& ctx) return temBAD_FEE; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/AMMDeposit.h b/src/xrpld/app/tx/detail/AMMDeposit.h index 0acb1dd9ab..c1a37be452 100644 --- a/src/xrpld/app/tx/detail/AMMDeposit.h +++ b/src/xrpld/app/tx/detail/AMMDeposit.h @@ -68,6 +68,12 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/AMMVote.cpp b/src/xrpld/app/tx/detail/AMMVote.cpp index 6fbff86056..0ffbb38b37 100644 --- a/src/xrpld/app/tx/detail/AMMVote.cpp +++ b/src/xrpld/app/tx/detail/AMMVote.cpp @@ -27,15 +27,15 @@ namespace ripple { +bool +AMMVote::checkExtraFeatures(PreflightContext const& ctx) +{ + return ammEnabled(ctx.rules); +} + NotTEC AMMVote::preflight(PreflightContext const& ctx) { - if (!ammEnabled(ctx.rules)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - if (auto const res = invalidAMMAssetPair( ctx.tx[sfAsset].get(), ctx.tx[sfAsset2].get())) { @@ -43,19 +43,13 @@ AMMVote::preflight(PreflightContext const& ctx) return res; } - if (ctx.tx.getFlags() & tfUniversalMask) - { - JLOG(ctx.j.debug()) << "AMM Vote: invalid flags."; - return temINVALID_FLAG; - } - if (ctx.tx[sfTradingFee] > TRADING_FEE_THRESHOLD) { JLOG(ctx.j.debug()) << "AMM Vote: invalid trading fee."; return temBAD_FEE; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/AMMVote.h b/src/xrpld/app/tx/detail/AMMVote.h index 2bee01aff5..dc99480111 100644 --- a/src/xrpld/app/tx/detail/AMMVote.h +++ b/src/xrpld/app/tx/detail/AMMVote.h @@ -56,6 +56,9 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.cpp b/src/xrpld/app/tx/detail/AMMWithdraw.cpp index f19318f28e..073cfd3ccf 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.cpp +++ b/src/xrpld/app/tx/detail/AMMWithdraw.cpp @@ -28,21 +28,22 @@ namespace ripple { +bool +AMMWithdraw::checkExtraFeatures(PreflightContext const& ctx) +{ + return ammEnabled(ctx.rules); +} + +std::uint32_t +AMMWithdraw::getFlagsMask(PreflightContext const& ctx) +{ + return tfWithdrawMask; +} + NotTEC AMMWithdraw::preflight(PreflightContext const& ctx) { - if (!ammEnabled(ctx.rules)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - auto const flags = ctx.tx.getFlags(); - if (flags & tfWithdrawMask) - { - JLOG(ctx.j.debug()) << "AMM Withdraw: invalid flags."; - return temINVALID_FLAG; - } auto const amount = ctx.tx[~sfAmount]; auto const amount2 = ctx.tx[~sfAmount2]; @@ -150,7 +151,7 @@ AMMWithdraw::preflight(PreflightContext const& ctx) } } - return preflight2(ctx); + return tesSUCCESS; } static std::optional diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.h b/src/xrpld/app/tx/detail/AMMWithdraw.h index 259a97eb89..1d07ea33da 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.h +++ b/src/xrpld/app/tx/detail/AMMWithdraw.h @@ -76,6 +76,12 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/Batch.cpp b/src/xrpld/app/tx/detail/Batch.cpp index 86d6e8a8f4..cba89348d0 100644 --- a/src/xrpld/app/tx/detail/Batch.cpp +++ b/src/xrpld/app/tx/detail/Batch.cpp @@ -164,6 +164,12 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx) return signerFees + txnFees + batchBase; } +std::uint32_t +Batch::getFlagsMask(PreflightContext const& ctx) +{ + return tfBatchMask; +} + /** * @brief Performs preflight validation checks for a Batch transaction. * @@ -200,23 +206,9 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx) NotTEC Batch::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureBatch)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - auto const parentBatchId = ctx.tx.getTransactionID(); - auto const outerAccount = ctx.tx.getAccountID(sfAccount); auto const flags = ctx.tx.getFlags(); - if (flags & tfBatchMask) - { - JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]:" - << "invalid flags."; - return temINVALID_FLAG; - } - if (std::popcount( flags & (tfAllOrNothing | tfOnlyOne | tfUntilFailure | tfIndependent)) != 1) @@ -242,7 +234,6 @@ Batch::preflight(PreflightContext const& ctx) } // Validation Inner Batch Txns - std::unordered_set requiredSigners; std::unordered_set uniqueHashes; std::unordered_map> accountSeqTicket; @@ -372,6 +363,23 @@ Batch::preflight(PreflightContext const& ctx) } } } + } + + return tesSUCCESS; +} + +NotTEC +Batch::preflightSigValidated(PreflightContext const& ctx) +{ + auto const parentBatchId = ctx.tx.getTransactionID(); + auto const outerAccount = ctx.tx.getAccountID(sfAccount); + auto const& rawTxns = ctx.tx.getFieldArray(sfRawTransactions); + + // Build the signers list + std::unordered_set requiredSigners; + for (STObject const& rb : rawTxns) + { + auto const innerAccount = rb.getAccountID(sfAccount); // If the inner account is the same as the outer account, do not add the // inner account to the required signers set. @@ -379,11 +387,6 @@ Batch::preflight(PreflightContext const& ctx) requiredSigners.insert(innerAccount); } - // LCOV_EXCL_START - if (auto const ret = preflight2(ctx); !isTesSuccess(ret)) - return ret; - // LCOV_EXCL_STOP - // Validation Batch Signers std::unordered_set batchSigners; if (ctx.tx.isFieldPresent(sfBatchSigners)) diff --git a/src/xrpld/app/tx/detail/Batch.h b/src/xrpld/app/tx/detail/Batch.h index 211bce0589..07863a5f33 100644 --- a/src/xrpld/app/tx/detail/Batch.h +++ b/src/xrpld/app/tx/detail/Batch.h @@ -40,9 +40,15 @@ public: static XRPAmount calculateBaseFee(ReadView const& view, STTx const& tx); + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); + static NotTEC + preflightSigValidated(PreflightContext const& ctx); + static NotTEC checkSign(PreclaimContext const& ctx); diff --git a/src/xrpld/app/tx/detail/CancelCheck.cpp b/src/xrpld/app/tx/detail/CancelCheck.cpp index 9bc79c8532..0393775dbd 100644 --- a/src/xrpld/app/tx/detail/CancelCheck.cpp +++ b/src/xrpld/app/tx/detail/CancelCheck.cpp @@ -32,21 +32,7 @@ namespace ripple { NotTEC CancelCheck::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureChecks)) - return temDISABLED; - - NotTEC const ret{preflight1(ctx)}; - if (!isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - { - // There are no flags (other than universal) for CreateCheck yet. - JLOG(ctx.j.warn()) << "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } - - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/CancelOffer.cpp b/src/xrpld/app/tx/detail/CancelOffer.cpp index e0a5c7baa7..e7ec28ce17 100644 --- a/src/xrpld/app/tx/detail/CancelOffer.cpp +++ b/src/xrpld/app/tx/detail/CancelOffer.cpp @@ -28,25 +28,13 @@ namespace ripple { NotTEC CancelOffer::preflight(PreflightContext const& ctx) { - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - auto const uTxFlags = ctx.tx.getFlags(); - - if (uTxFlags & tfUniversalMask) - { - JLOG(ctx.j.trace()) << "Malformed transaction: " - << "Invalid flags set."; - return temINVALID_FLAG; - } - if (!ctx.tx[sfOfferSequence]) { JLOG(ctx.j.trace()) << "CancelOffer::preflight: missing sequence"; return temBAD_SEQUENCE; } - return preflight2(ctx); + return tesSUCCESS; } //------------------------------------------------------------------------------ diff --git a/src/xrpld/app/tx/detail/CashCheck.cpp b/src/xrpld/app/tx/detail/CashCheck.cpp index 8278b16649..98eaa4f0e8 100644 --- a/src/xrpld/app/tx/detail/CashCheck.cpp +++ b/src/xrpld/app/tx/detail/CashCheck.cpp @@ -35,20 +35,6 @@ namespace ripple { NotTEC CashCheck::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureChecks)) - return temDISABLED; - - NotTEC const ret{preflight1(ctx)}; - if (!isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - { - // There are no flags (other than universal) for CashCheck yet. - JLOG(ctx.j.warn()) << "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } - // Exactly one of Amount or DeliverMin must be present. auto const optAmount = ctx.tx[~sfAmount]; auto const optDeliverMin = ctx.tx[~sfDeliverMin]; @@ -76,7 +62,7 @@ CashCheck::preflight(PreflightContext const& ctx) return temBAD_CURRENCY; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/Change.cpp b/src/xrpld/app/tx/detail/Change.cpp index 60720ba2f6..26fa354119 100644 --- a/src/xrpld/app/tx/detail/Change.cpp +++ b/src/xrpld/app/tx/detail/Change.cpp @@ -33,11 +33,12 @@ namespace ripple { +template <> NotTEC -Change::preflight(PreflightContext const& ctx) +Transactor::invokePreflight(PreflightContext const& ctx) { - auto const ret = preflight0(ctx); - if (!isTesSuccess(ret)) + // 0 means "Allow any flags" + if (auto const ret = preflight0(ctx, 0)) return ret; auto account = ctx.tx.getAccountID(sfAccount); diff --git a/src/xrpld/app/tx/detail/Change.h b/src/xrpld/app/tx/detail/Change.h index d710827dd6..7b8fbf3421 100644 --- a/src/xrpld/app/tx/detail/Change.h +++ b/src/xrpld/app/tx/detail/Change.h @@ -33,9 +33,6 @@ public: { } - static NotTEC - preflight(PreflightContext const& ctx); - TER doApply() override; void diff --git a/src/xrpld/app/tx/detail/Clawback.cpp b/src/xrpld/app/tx/detail/Clawback.cpp index 08cf4baef0..b346e4a1c1 100644 --- a/src/xrpld/app/tx/detail/Clawback.cpp +++ b/src/xrpld/app/tx/detail/Clawback.cpp @@ -75,25 +75,22 @@ preflightHelper(PreflightContext const& ctx) return tesSUCCESS; } +std::uint32_t +Clawback::getFlagsMask(PreflightContext const& ctx) +{ + return tfClawbackMask; +} + NotTEC Clawback::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureClawback)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfClawbackMask) - return temINVALID_FLAG; - if (auto const ret = std::visit( [&](T const&) { return preflightHelper(ctx); }, ctx.tx[sfAmount].asset().value()); !isTesSuccess(ret)) return ret; - return preflight2(ctx); + return tesSUCCESS; } template diff --git a/src/xrpld/app/tx/detail/Clawback.h b/src/xrpld/app/tx/detail/Clawback.h index d908a2e4ef..b02233c2ed 100644 --- a/src/xrpld/app/tx/detail/Clawback.h +++ b/src/xrpld/app/tx/detail/Clawback.h @@ -33,6 +33,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/CreateCheck.cpp b/src/xrpld/app/tx/detail/CreateCheck.cpp index d0e5caebdb..80e6aa2026 100644 --- a/src/xrpld/app/tx/detail/CreateCheck.cpp +++ b/src/xrpld/app/tx/detail/CreateCheck.cpp @@ -31,19 +31,6 @@ namespace ripple { NotTEC CreateCheck::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureChecks)) - return temDISABLED; - - NotTEC const ret{preflight1(ctx)}; - if (!isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - { - // There are no flags (other than universal) for CreateCheck yet. - JLOG(ctx.j.warn()) << "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } if (ctx.tx[sfAccount] == ctx.tx[sfDestination]) { // They wrote a check to themselves. @@ -76,7 +63,7 @@ CreateCheck::preflight(PreflightContext const& ctx) } } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/CreateOffer.cpp b/src/xrpld/app/tx/detail/CreateOffer.cpp index 46e1184ec9..b101e08e03 100644 --- a/src/xrpld/app/tx/detail/CreateOffer.cpp +++ b/src/xrpld/app/tx/detail/CreateOffer.cpp @@ -43,30 +43,36 @@ CreateOffer::makeTxConsequences(PreflightContext const& ctx) return TxConsequences{ctx.tx, calculateMaxXRPSpend(ctx.tx)}; } -NotTEC -CreateOffer::preflight(PreflightContext const& ctx) +bool +CreateOffer::checkExtraFeatures(PreflightContext const& ctx) { if (ctx.tx.isFieldPresent(sfDomainID) && !ctx.rules.enabled(featurePermissionedDEX)) - return temDISABLED; + return false; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; + return true; +} +std::uint32_t +CreateOffer::getFlagsMask(PreflightContext const& ctx) +{ + // The tfOfferCreateMask is built assuming that PermissionedDEX is + // enabled + if (ctx.rules.enabled(featurePermissionedDEX)) + return tfOfferCreateMask; + // If PermissionedDEX is not enabled, add tfHybrid to the mask, + // indicating it is not allowed. + return tfOfferCreateMask | tfHybrid; +} + +NotTEC +CreateOffer::preflight(PreflightContext const& ctx) +{ auto& tx = ctx.tx; auto& j = ctx.j; std::uint32_t const uTxFlags = tx.getFlags(); - if (uTxFlags & tfOfferCreateMask) - { - JLOG(j.debug()) << "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } - - if (!ctx.rules.enabled(featurePermissionedDEX) && tx.isFlag(tfHybrid)) - return temINVALID_FLAG; - if (tx.isFlag(tfHybrid) && !tx.isFieldPresent(sfDomainID)) return temINVALID_FLAG; @@ -136,7 +142,7 @@ CreateOffer::preflight(PreflightContext const& ctx) return temBAD_ISSUER; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/CreateOffer.h b/src/xrpld/app/tx/detail/CreateOffer.h index 6e3d6145b1..c38e244b34 100644 --- a/src/xrpld/app/tx/detail/CreateOffer.h +++ b/src/xrpld/app/tx/detail/CreateOffer.h @@ -43,6 +43,12 @@ public: static TxConsequences makeTxConsequences(PreflightContext const& ctx); + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + /** Enforce constraints beyond those of the Transactor base class. */ static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/CreateTicket.cpp b/src/xrpld/app/tx/detail/CreateTicket.cpp index e2b97cecd4..d2821626b2 100644 --- a/src/xrpld/app/tx/detail/CreateTicket.cpp +++ b/src/xrpld/app/tx/detail/CreateTicket.cpp @@ -36,20 +36,11 @@ CreateTicket::makeTxConsequences(PreflightContext const& ctx) NotTEC CreateTicket::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureTicketBatch)) - return temDISABLED; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (std::uint32_t const count = ctx.tx[sfTicketCount]; count < minValidCount || count > maxValidCount) return temINVALID_COUNT; - if (NotTEC const ret{preflight1(ctx)}; !isTesSuccess(ret)) - return ret; - - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/Credentials.cpp b/src/xrpld/app/tx/detail/Credentials.cpp index c5970bb054..dbde4caa1a 100644 --- a/src/xrpld/app/tx/detail/Credentials.cpp +++ b/src/xrpld/app/tx/detail/Credentials.cpp @@ -48,28 +48,19 @@ using namespace credentials; // ------- CREATE -------------------------- +std::uint32_t +CredentialCreate::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fixInvalidTxFlags) ? tfUniversalMask : 0; +} + NotTEC CredentialCreate::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureCredentials)) - { - JLOG(ctx.j.trace()) << "featureCredentials is disabled."; - return temDISABLED; - } - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - auto const& tx = ctx.tx; auto& j = ctx.j; - if (ctx.rules.enabled(fixInvalidTxFlags) && - (tx.getFlags() & tfUniversalMask)) - { - JLOG(ctx.j.debug()) << "CredentialCreate: invalid flags."; - return temINVALID_FLAG; - } - if (!tx[sfSubject]) { JLOG(j.trace()) << "Malformed transaction: Invalid Subject"; @@ -91,7 +82,7 @@ CredentialCreate::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER @@ -202,25 +193,17 @@ CredentialCreate::doApply() } // ------- DELETE -------------------------- + +std::uint32_t +CredentialDelete::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fixInvalidTxFlags) ? tfUniversalMask : 0; +} + NotTEC CredentialDelete::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureCredentials)) - { - JLOG(ctx.j.trace()) << "featureCredentials is disabled."; - return temDISABLED; - } - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.rules.enabled(fixInvalidTxFlags) && - (ctx.tx.getFlags() & tfUniversalMask)) - { - JLOG(ctx.j.debug()) << "CredentialDelete: invalid flags."; - return temINVALID_FLAG; - } - auto const subject = ctx.tx[~sfSubject]; auto const issuer = ctx.tx[~sfIssuer]; @@ -248,7 +231,7 @@ CredentialDelete::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER @@ -289,25 +272,16 @@ CredentialDelete::doApply() // ------- APPLY -------------------------- +std::uint32_t +CredentialAccept::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fixInvalidTxFlags) ? tfUniversalMask : 0; +} + NotTEC CredentialAccept::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureCredentials)) - { - JLOG(ctx.j.trace()) << "featureCredentials is disabled."; - return temDISABLED; - } - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.rules.enabled(fixInvalidTxFlags) && - (ctx.tx.getFlags() & tfUniversalMask)) - { - JLOG(ctx.j.debug()) << "CredentialAccept: invalid flags."; - return temINVALID_FLAG; - } - if (!ctx.tx[sfIssuer]) { JLOG(ctx.j.trace()) << "Malformed transaction: Issuer field zeroed."; @@ -322,7 +296,7 @@ CredentialAccept::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/Credentials.h b/src/xrpld/app/tx/detail/Credentials.h index 5b4acb3998..a5885a2226 100644 --- a/src/xrpld/app/tx/detail/Credentials.h +++ b/src/xrpld/app/tx/detail/Credentials.h @@ -33,6 +33,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); @@ -54,6 +57,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); @@ -75,6 +81,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/DID.cpp b/src/xrpld/app/tx/detail/DID.cpp index 9ac461b2e8..fb0388940d 100644 --- a/src/xrpld/app/tx/detail/DID.cpp +++ b/src/xrpld/app/tx/detail/DID.cpp @@ -45,15 +45,6 @@ namespace ripple { NotTEC DIDSet::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureDID)) - return temDISABLED; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - if (!ctx.tx.isFieldPresent(sfURI) && !ctx.tx.isFieldPresent(sfDIDDocument) && !ctx.tx.isFieldPresent(sfData)) return temEMPTY_DID; @@ -74,7 +65,7 @@ DIDSet::preflight(PreflightContext const& ctx) isTooLong(sfData, maxDIDAttestationLength)) return temMALFORMED; - return preflight2(ctx); + return tesSUCCESS; } TER @@ -173,16 +164,7 @@ DIDSet::doApply() NotTEC DIDDelete::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureDID)) - return temDISABLED; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/DelegateSet.cpp b/src/xrpld/app/tx/detail/DelegateSet.cpp index decc9333f4..2bc666370a 100644 --- a/src/xrpld/app/tx/detail/DelegateSet.cpp +++ b/src/xrpld/app/tx/detail/DelegateSet.cpp @@ -30,12 +30,6 @@ namespace ripple { NotTEC DelegateSet::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featurePermissionDelegation)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - auto const& permissions = ctx.tx.getFieldArray(sfPermissions); if (permissions.size() > permissionMaxSize) return temARRAY_TOO_LARGE; @@ -57,7 +51,7 @@ DelegateSet::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/DeleteAccount.cpp b/src/xrpld/app/tx/detail/DeleteAccount.cpp index 18f8fd1adf..80aee70f79 100644 --- a/src/xrpld/app/tx/detail/DeleteAccount.cpp +++ b/src/xrpld/app/tx/detail/DeleteAccount.cpp @@ -39,22 +39,22 @@ namespace ripple { -NotTEC -DeleteAccount::preflight(PreflightContext const& ctx) +bool +DeleteAccount::checkExtraFeatures(PreflightContext const& ctx) { if (!ctx.rules.enabled(featureDeletableAccounts)) - return temDISABLED; + return false; if (ctx.tx.isFieldPresent(sfCredentialIDs) && !ctx.rules.enabled(featureCredentials)) - return temDISABLED; + return false; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; + return true; +} +NotTEC +DeleteAccount::preflight(PreflightContext const& ctx) +{ if (ctx.tx[sfAccount] == ctx.tx[sfDestination]) // An account cannot be deleted and give itself the resulting XRP. return temDST_IS_SRC; @@ -63,14 +63,14 @@ DeleteAccount::preflight(PreflightContext const& ctx) !isTesSuccess(err)) return err; - return preflight2(ctx); + return tesSUCCESS; } XRPAmount DeleteAccount::calculateBaseFee(ReadView const& view, STTx const& tx) { // The fee required for AccountDelete is one owner reserve. - return view.fees().increment; + return calculateOwnerReserveFee(view, tx); } namespace { diff --git a/src/xrpld/app/tx/detail/DeleteAccount.h b/src/xrpld/app/tx/detail/DeleteAccount.h index c9d3305562..ee9db97d50 100644 --- a/src/xrpld/app/tx/detail/DeleteAccount.h +++ b/src/xrpld/app/tx/detail/DeleteAccount.h @@ -33,6 +33,9 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/DeleteOracle.cpp b/src/xrpld/app/tx/detail/DeleteOracle.cpp index e1e8a5d6f1..e84cc2c333 100644 --- a/src/xrpld/app/tx/detail/DeleteOracle.cpp +++ b/src/xrpld/app/tx/detail/DeleteOracle.cpp @@ -29,19 +29,7 @@ namespace ripple { NotTEC DeleteOracle::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featurePriceOracle)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - { - JLOG(ctx.j.debug()) << "Oracle Delete: invalid flags."; - return temINVALID_FLAG; - } - - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/DepositPreauth.cpp b/src/xrpld/app/tx/detail/DepositPreauth.cpp index bd9698eba3..7a5a4b7e02 100644 --- a/src/xrpld/app/tx/detail/DepositPreauth.cpp +++ b/src/xrpld/app/tx/detail/DepositPreauth.cpp @@ -30,32 +30,29 @@ namespace ripple { +bool +DepositPreauth::checkExtraFeatures(PreflightContext const& ctx) +{ + bool const authArrPresent = ctx.tx.isFieldPresent(sfAuthorizeCredentials); + bool const unauthArrPresent = + ctx.tx.isFieldPresent(sfUnauthorizeCredentials); + bool const authCredPresent = authArrPresent || unauthArrPresent; + + if (authCredPresent && !ctx.rules.enabled(featureCredentials)) + return false; + + return true; +} + NotTEC DepositPreauth::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureDepositPreauth)) - return temDISABLED; - bool const authArrPresent = ctx.tx.isFieldPresent(sfAuthorizeCredentials); bool const unauthArrPresent = ctx.tx.isFieldPresent(sfUnauthorizeCredentials); int const authCredPresent = static_cast(authArrPresent) + static_cast(unauthArrPresent); - if (authCredPresent && !ctx.rules.enabled(featureCredentials)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - auto& tx = ctx.tx; - - if (tx.getFlags() & tfUniversalMask) - { - JLOG(ctx.j.trace()) << "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } - auto const optAuth = ctx.tx[~sfAuthorize]; auto const optUnauth = ctx.tx[~sfUnauthorize]; int const authPresent = static_cast(optAuth.has_value()) + @@ -102,7 +99,7 @@ DepositPreauth::preflight(PreflightContext const& ctx) return err; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/DepositPreauth.h b/src/xrpld/app/tx/detail/DepositPreauth.h index 76a7c08073..ead17742cd 100644 --- a/src/xrpld/app/tx/detail/DepositPreauth.h +++ b/src/xrpld/app/tx/detail/DepositPreauth.h @@ -33,6 +33,9 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/Escrow.cpp b/src/xrpld/app/tx/detail/Escrow.cpp index e163186eee..22a22db1b6 100644 --- a/src/xrpld/app/tx/detail/Escrow.cpp +++ b/src/xrpld/app/tx/detail/Escrow.cpp @@ -118,15 +118,16 @@ escrowCreatePreflightHelper(PreflightContext const& ctx) return tesSUCCESS; } +std::uint32_t +EscrowCreate::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fix1543) ? tfUniversalMask : 0; +} + NotTEC EscrowCreate::preflight(PreflightContext const& ctx) { - if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - STAmount const amount{ctx.tx[sfAmount]}; if (!isXRP(amount)) { @@ -189,7 +190,7 @@ EscrowCreate::preflight(PreflightContext const& ctx) return temDISABLED; } - return preflight2(ctx); + return tesSUCCESS; } template @@ -635,19 +636,23 @@ checkCondition(Slice f, Slice c) return validate(*fulfillment, *condition); } +bool +EscrowFinish::checkExtraFeatures(PreflightContext const& ctx) +{ + return !ctx.tx.isFieldPresent(sfCredentialIDs) || + ctx.rules.enabled(featureCredentials); +} + +std::uint32_t +EscrowFinish::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fix1543) ? tfUniversalMask : 0; +} + NotTEC EscrowFinish::preflight(PreflightContext const& ctx) { - if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - if (ctx.tx.isFieldPresent(sfCredentialIDs) && - !ctx.rules.enabled(featureCredentials)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - auto const cb = ctx.tx[~sfCondition]; auto const fb = ctx.tx[~sfFulfillment]; @@ -656,13 +661,14 @@ EscrowFinish::preflight(PreflightContext const& ctx) if (static_cast(cb) != static_cast(fb)) return temMALFORMED; - // Verify the transaction signature. If it doesn't work - // then don't do any more work. - { - auto const ret = preflight2(ctx); - if (!isTesSuccess(ret)) - return ret; - } + return tesSUCCESS; +} + +NotTEC +EscrowFinish::preflightSigValidated(PreflightContext const& ctx) +{ + auto const cb = ctx.tx[~sfCondition]; + auto const fb = ctx.tx[~sfFulfillment]; if (cb && fb) { @@ -1225,16 +1231,17 @@ EscrowFinish::doApply() //------------------------------------------------------------------------------ +std::uint32_t +EscrowCancel::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fix1543) ? tfUniversalMask : 0; +} + NotTEC EscrowCancel::preflight(PreflightContext const& ctx) { - if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - return preflight2(ctx); + return tesSUCCESS; } template diff --git a/src/xrpld/app/tx/detail/Escrow.h b/src/xrpld/app/tx/detail/Escrow.h index 2225c94f16..8956be2939 100644 --- a/src/xrpld/app/tx/detail/Escrow.h +++ b/src/xrpld/app/tx/detail/Escrow.h @@ -36,6 +36,9 @@ public: static TxConsequences makeTxConsequences(PreflightContext const& ctx); + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); @@ -57,9 +60,18 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); + static NotTEC + preflightSigValidated(PreflightContext const& ctx); + static XRPAmount calculateBaseFee(ReadView const& view, STTx const& tx); @@ -81,6 +93,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/LedgerStateFix.cpp b/src/xrpld/app/tx/detail/LedgerStateFix.cpp index b861f1d0ef..6059e15313 100644 --- a/src/xrpld/app/tx/detail/LedgerStateFix.cpp +++ b/src/xrpld/app/tx/detail/LedgerStateFix.cpp @@ -30,15 +30,6 @@ namespace ripple { NotTEC LedgerStateFix::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(fixNFTokenPageLinks)) - return temDISABLED; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - switch (ctx.tx[sfLedgerFixType]) { case FixType::nfTokenPageLink: @@ -50,7 +41,7 @@ LedgerStateFix::preflight(PreflightContext const& ctx) return tefINVALID_LEDGER_FIX_TYPE; } - return preflight2(ctx); + return tesSUCCESS; } XRPAmount @@ -58,7 +49,7 @@ LedgerStateFix::calculateBaseFee(ReadView const& view, STTx const& tx) { // The fee required for LedgerStateFix is one owner reserve, just like // the fee for AccountDelete. - return view.fees().increment; + return calculateOwnerReserveFee(view, tx); } TER diff --git a/src/xrpld/app/tx/detail/MPTokenAuthorize.cpp b/src/xrpld/app/tx/detail/MPTokenAuthorize.cpp index af05b08beb..4a3ac04972 100644 --- a/src/xrpld/app/tx/detail/MPTokenAuthorize.cpp +++ b/src/xrpld/app/tx/detail/MPTokenAuthorize.cpp @@ -26,22 +26,19 @@ namespace ripple { +std::uint32_t +MPTokenAuthorize::getFlagsMask(PreflightContext const& ctx) +{ + return tfMPTokenAuthorizeMask; +} + NotTEC MPTokenAuthorize::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureMPTokensV1)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfMPTokenAuthorizeMask) - return temINVALID_FLAG; - if (ctx.tx[sfAccount] == ctx.tx[~sfHolder]) return temMALFORMED; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/MPTokenAuthorize.h b/src/xrpld/app/tx/detail/MPTokenAuthorize.h index 85e8edcf9f..43a962e24e 100644 --- a/src/xrpld/app/tx/detail/MPTokenAuthorize.h +++ b/src/xrpld/app/tx/detail/MPTokenAuthorize.h @@ -42,6 +42,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.cpp b/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.cpp index cd0d9b1ca7..60fde01c93 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.cpp +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.cpp @@ -25,31 +25,37 @@ namespace ripple { -NotTEC -MPTokenIssuanceCreate::preflight(PreflightContext const& ctx) +bool +MPTokenIssuanceCreate::checkExtraFeatures(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureMPTokensV1)) - return temDISABLED; - if (ctx.tx.isFieldPresent(sfDomainID) && !(ctx.rules.enabled(featurePermissionedDomains) && ctx.rules.enabled(featureSingleAssetVault))) - return temDISABLED; + return false; if (ctx.tx.isFieldPresent(sfMutableFlags) && !ctx.rules.enabled(featureDynamicMPT)) - return temDISABLED; + return false; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; + return true; +} +std::uint32_t +MPTokenIssuanceCreate::getFlagsMask(PreflightContext const& ctx) +{ + // This mask is only compared against sfFlags + return tfMPTokenIssuanceCreateMask; +} + +NotTEC +MPTokenIssuanceCreate::preflight(PreflightContext const& ctx) +{ + // If the mutable flags field is included, at least one flag must be + // specified. if (auto const mutableFlags = ctx.tx[~sfMutableFlags]; mutableFlags && (!*mutableFlags || *mutableFlags & tmfMPTokenIssuanceCreateMutableMask)) return temINVALID_FLAG; - if (ctx.tx.getFlags() & tfMPTokenIssuanceCreateMask) - return temINVALID_FLAG; - if (auto const fee = ctx.tx[~sfTransferFee]) { if (fee > maxTransferFee) @@ -87,7 +93,7 @@ MPTokenIssuanceCreate::preflight(PreflightContext const& ctx) if (maxAmt > maxMPTokenAmount) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } Expected diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.h b/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.h index 941c1ecef7..6120e567bc 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.h +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.h @@ -50,6 +50,12 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.cpp b/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.cpp index 2e8f1537c5..928f330baa 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.cpp +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.cpp @@ -25,20 +25,16 @@ namespace ripple { +std::uint32_t +MPTokenIssuanceDestroy::getFlagsMask(PreflightContext const& ctx) +{ + return tfMPTokenIssuanceDestroyMask; +} + NotTEC MPTokenIssuanceDestroy::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureMPTokensV1)) - return temDISABLED; - - // check flags - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfMPTokenIssuanceDestroyMask) - return temINVALID_FLAG; - - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.h b/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.h index 69abb99feb..2cebdb7352 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.h +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.h @@ -33,6 +33,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp index 37c563460a..c406a8ec5f 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp @@ -26,6 +26,20 @@ namespace ripple { +bool +MPTokenIssuanceSet::checkExtraFeatures(PreflightContext const& ctx) +{ + return !ctx.tx.isFieldPresent(sfDomainID) || + (ctx.rules.enabled(featurePermissionedDomains) && + ctx.rules.enabled(featureSingleAssetVault)); +} + +std::uint32_t +MPTokenIssuanceSet::getFlagsMask(PreflightContext const& ctx) +{ + return tfMPTokenIssuanceSetMask; +} + // Maps set/clear mutable flags in an MPTokenIssuanceSet transaction to the // corresponding ledger mutable flags that control whether the change is // allowed. @@ -49,14 +63,6 @@ static constexpr std::array mptMutabilityFlags = { NotTEC MPTokenIssuanceSet::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureMPTokensV1)) - return temDISABLED; - - if (ctx.tx.isFieldPresent(sfDomainID) && - !(ctx.rules.enabled(featurePermissionedDomains) && - ctx.rules.enabled(featureSingleAssetVault))) - return temDISABLED; - auto const mutableFlags = ctx.tx[~sfMutableFlags]; auto const metadata = ctx.tx[~sfMPTokenMetadata]; auto const transferFee = ctx.tx[~sfTransferFee]; @@ -68,16 +74,10 @@ MPTokenIssuanceSet::preflight(PreflightContext const& ctx) if (ctx.tx.isFieldPresent(sfDomainID) && ctx.tx.isFieldPresent(sfHolder)) return temMALFORMED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - auto const txFlags = ctx.tx.getFlags(); - // check flags - if (txFlags & tfMPTokenIssuanceSetMask) - return temINVALID_FLAG; // fails if both flags are set - else if ((txFlags & tfMPTLock) && (txFlags & tfMPTUnlock)) + if ((txFlags & tfMPTLock) && (txFlags & tfMPTUnlock)) return temINVALID_FLAG; auto const accountID = ctx.tx[sfAccount]; @@ -133,7 +133,7 @@ MPTokenIssuanceSet::preflight(PreflightContext const& ctx) } } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.h b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.h index 5b3db0e75b..f63812097e 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.h +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.h @@ -33,6 +33,12 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp b/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp index f522ceaffa..681e18bee1 100644 --- a/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp +++ b/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp @@ -27,18 +27,15 @@ namespace ripple { +std::uint32_t +NFTokenAcceptOffer::getFlagsMask(PreflightContext const& ctx) +{ + return tfNFTokenAcceptOfferMask; +} + NotTEC NFTokenAcceptOffer::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureNonFungibleTokensV1)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfNFTokenAcceptOfferMask) - return temINVALID_FLAG; - auto const bo = ctx.tx[~sfNFTokenBuyOffer]; auto const so = ctx.tx[~sfNFTokenSellOffer]; @@ -57,7 +54,7 @@ NFTokenAcceptOffer::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/NFTokenAcceptOffer.h b/src/xrpld/app/tx/detail/NFTokenAcceptOffer.h index dff3febbb2..995581d1ff 100644 --- a/src/xrpld/app/tx/detail/NFTokenAcceptOffer.h +++ b/src/xrpld/app/tx/detail/NFTokenAcceptOffer.h @@ -51,6 +51,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/NFTokenBurn.cpp b/src/xrpld/app/tx/detail/NFTokenBurn.cpp index 947a663f92..cb1b564402 100644 --- a/src/xrpld/app/tx/detail/NFTokenBurn.cpp +++ b/src/xrpld/app/tx/detail/NFTokenBurn.cpp @@ -29,16 +29,7 @@ namespace ripple { NotTEC NFTokenBurn::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureNonFungibleTokensV1)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/NFTokenCancelOffer.cpp b/src/xrpld/app/tx/detail/NFTokenCancelOffer.cpp index 3d0bf04a1b..86e804b1a5 100644 --- a/src/xrpld/app/tx/detail/NFTokenCancelOffer.cpp +++ b/src/xrpld/app/tx/detail/NFTokenCancelOffer.cpp @@ -28,18 +28,15 @@ namespace ripple { +std::uint32_t +NFTokenCancelOffer::getFlagsMask(PreflightContext const& ctx) +{ + return tfNFTokenCancelOfferMask; +} + NotTEC NFTokenCancelOffer::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureNonFungibleTokensV1)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfNFTokenCancelOfferMask) - return temINVALID_FLAG; - if (auto const& ids = ctx.tx[sfNFTokenOffers]; ids.empty() || (ids.size() > maxTokenOfferCancelCount)) return temMALFORMED; @@ -51,7 +48,7 @@ NFTokenCancelOffer::preflight(PreflightContext const& ctx) if (std::adjacent_find(ids.begin(), ids.end()) != ids.end()) return temMALFORMED; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/NFTokenCancelOffer.h b/src/xrpld/app/tx/detail/NFTokenCancelOffer.h index d460675711..b35be0e757 100644 --- a/src/xrpld/app/tx/detail/NFTokenCancelOffer.h +++ b/src/xrpld/app/tx/detail/NFTokenCancelOffer.h @@ -33,6 +33,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/NFTokenCreateOffer.cpp b/src/xrpld/app/tx/detail/NFTokenCreateOffer.cpp index 6ff14e33bd..22f825d352 100644 --- a/src/xrpld/app/tx/detail/NFTokenCreateOffer.cpp +++ b/src/xrpld/app/tx/detail/NFTokenCreateOffer.cpp @@ -26,20 +26,17 @@ namespace ripple { +std::uint32_t +NFTokenCreateOffer::getFlagsMask(PreflightContext const& ctx) +{ + return tfNFTokenCreateOfferMask; +} + NotTEC NFTokenCreateOffer::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureNonFungibleTokensV1)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - auto const txFlags = ctx.tx.getFlags(); - if (txFlags & tfNFTokenCreateOfferMask) - return temINVALID_FLAG; - auto const nftFlags = nft::getFlags(ctx.tx[sfNFTokenID]); // Use implementation shared with NFTokenMint @@ -55,7 +52,7 @@ NFTokenCreateOffer::preflight(PreflightContext const& ctx) !isTesSuccess(notTec)) return notTec; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/NFTokenCreateOffer.h b/src/xrpld/app/tx/detail/NFTokenCreateOffer.h index 075a5a712f..0a1c631298 100644 --- a/src/xrpld/app/tx/detail/NFTokenCreateOffer.h +++ b/src/xrpld/app/tx/detail/NFTokenCreateOffer.h @@ -33,6 +33,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/NFTokenMint.cpp b/src/xrpld/app/tx/detail/NFTokenMint.cpp index b3fa8cfec5..05f1e75926 100644 --- a/src/xrpld/app/tx/detail/NFTokenMint.cpp +++ b/src/xrpld/app/tx/detail/NFTokenMint.cpp @@ -38,22 +38,23 @@ extractNFTokenFlagsFromTxFlags(std::uint32_t txFlags) return static_cast(txFlags & 0x0000FFFF); } -NotTEC -NFTokenMint::preflight(PreflightContext const& ctx) +static bool +hasOfferFields(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureNonFungibleTokensV1)) - return temDISABLED; - - bool const hasOfferFields = ctx.tx.isFieldPresent(sfAmount) || + return ctx.tx.isFieldPresent(sfAmount) || ctx.tx.isFieldPresent(sfDestination) || ctx.tx.isFieldPresent(sfExpiration); +} - if (!ctx.rules.enabled(featureNFTokenMintOffer) && hasOfferFields) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; +bool +NFTokenMint::checkExtraFeatures(PreflightContext const& ctx) +{ + return ctx.rules.enabled(featureNFTokenMintOffer) || !hasOfferFields(ctx); +} +std::uint32_t +NFTokenMint::getFlagsMask(PreflightContext const& ctx) +{ // Prior to fixRemoveNFTokenAutoTrustLine, transfer of an NFToken between // accounts allowed a TrustLine to be added to the issuer of that token // without explicit permission from that issuer. This was enabled by @@ -67,7 +68,7 @@ NFTokenMint::preflight(PreflightContext const& ctx) // The fixRemoveNFTokenAutoTrustLine amendment disables minting with the // tfTrustLine flag as a way to prevent the attack. But until the // amendment passes we still need to keep the old behavior available. - std::uint32_t const NFTokenMintMask = + std::uint32_t const nfTokenMintMask = ctx.rules.enabled(fixRemoveNFTokenAutoTrustLine) // if featureDynamicNFT enabled then new flag allowing mutable URI // available @@ -76,9 +77,12 @@ NFTokenMint::preflight(PreflightContext const& ctx) : ctx.rules.enabled(featureDynamicNFT) ? tfNFTokenMintOldMaskWithMutable : tfNFTokenMintOldMask; - if (ctx.tx.getFlags() & NFTokenMintMask) - return temINVALID_FLAG; + return nfTokenMintMask; +} +NotTEC +NFTokenMint::preflight(PreflightContext const& ctx) +{ if (auto const f = ctx.tx[~sfTransferFee]) { if (f > maxTransferFee) @@ -100,7 +104,7 @@ NFTokenMint::preflight(PreflightContext const& ctx) return temMALFORMED; } - if (hasOfferFields) + if (hasOfferFields(ctx)) { // The Amount field must be present if either the Destination or // Expiration fields are present. @@ -123,7 +127,7 @@ NFTokenMint::preflight(PreflightContext const& ctx) } } - return preflight2(ctx); + return tesSUCCESS; } uint256 diff --git a/src/xrpld/app/tx/detail/NFTokenMint.h b/src/xrpld/app/tx/detail/NFTokenMint.h index f606120c54..1606514559 100644 --- a/src/xrpld/app/tx/detail/NFTokenMint.h +++ b/src/xrpld/app/tx/detail/NFTokenMint.h @@ -36,6 +36,12 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/NFTokenModify.cpp b/src/xrpld/app/tx/detail/NFTokenModify.cpp index a3803c423b..6ae095411b 100644 --- a/src/xrpld/app/tx/detail/NFTokenModify.cpp +++ b/src/xrpld/app/tx/detail/NFTokenModify.cpp @@ -25,19 +25,15 @@ namespace ripple { +bool +NFTokenModify::checkExtraFeatures(PreflightContext const& ctx) +{ + return ctx.rules.enabled(featureNonFungibleTokensV1_1); +} + NotTEC NFTokenModify::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureNonFungibleTokensV1_1) || - !ctx.rules.enabled(featureDynamicNFT)) - return temDISABLED; - - if (NotTEC const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (auto owner = ctx.tx[~sfOwner]; owner == ctx.tx[sfAccount]) return temMALFORMED; @@ -47,7 +43,7 @@ NFTokenModify::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/NFTokenModify.h b/src/xrpld/app/tx/detail/NFTokenModify.h index 0d1e72ade1..04784381fb 100644 --- a/src/xrpld/app/tx/detail/NFTokenModify.h +++ b/src/xrpld/app/tx/detail/NFTokenModify.h @@ -33,6 +33,9 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/PayChan.cpp b/src/xrpld/app/tx/detail/PayChan.cpp index 0967e4665a..20f1ecae90 100644 --- a/src/xrpld/app/tx/detail/PayChan.cpp +++ b/src/xrpld/app/tx/detail/PayChan.cpp @@ -172,15 +172,16 @@ PayChanCreate::makeTxConsequences(PreflightContext const& ctx) return TxConsequences{ctx.tx, ctx.tx[sfAmount].xrp()}; } +std::uint32_t +PayChanCreate::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fix1543) ? tfUniversalMask : 0; +} + NotTEC PayChanCreate::preflight(PreflightContext const& ctx) { - if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - if (!isXRP(ctx.tx[sfAmount]) || (ctx.tx[sfAmount] <= beast::zero)) return temBAD_AMOUNT; @@ -190,7 +191,7 @@ PayChanCreate::preflight(PreflightContext const& ctx) if (!publicKeyType(ctx.tx[sfPublicKey])) return temMALFORMED; - return preflight2(ctx); + return tesSUCCESS; } TER @@ -336,19 +337,20 @@ PayChanFund::makeTxConsequences(PreflightContext const& ctx) return TxConsequences{ctx.tx, ctx.tx[sfAmount].xrp()}; } +std::uint32_t +PayChanFund::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fix1543) ? tfUniversalMask : 0; +} + NotTEC PayChanFund::preflight(PreflightContext const& ctx) { - if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - if (!isXRP(ctx.tx[sfAmount]) || (ctx.tx[sfAmount] <= beast::zero)) return temBAD_AMOUNT; - return preflight2(ctx); + return tesSUCCESS; } TER @@ -432,16 +434,23 @@ PayChanFund::doApply() //------------------------------------------------------------------------------ +bool +PayChanClaim::checkExtraFeatures(PreflightContext const& ctx) +{ + return !ctx.tx.isFieldPresent(sfCredentialIDs) || + ctx.rules.enabled(featureCredentials); +} + +std::uint32_t +PayChanClaim::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fix1543) ? tfPayChanClaimMask : 0; +} + NotTEC PayChanClaim::preflight(PreflightContext const& ctx) { - if (ctx.tx.isFieldPresent(sfCredentialIDs) && - !ctx.rules.enabled(featureCredentials)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - auto const bal = ctx.tx[~sfBalance]; if (bal && (!isXRP(*bal) || *bal <= beast::zero)) return temBAD_AMOUNT; @@ -456,9 +465,6 @@ PayChanClaim::preflight(PreflightContext const& ctx) { auto const flags = ctx.tx.getFlags(); - if (ctx.rules.enabled(fix1543) && (flags & tfPayChanClaimMask)) - return temINVALID_FLAG; - if ((flags & tfClose) && (flags & tfRenew)) return temMALFORMED; } @@ -493,7 +499,7 @@ PayChanClaim::preflight(PreflightContext const& ctx) !isTesSuccess(err)) return err; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/PayChan.h b/src/xrpld/app/tx/detail/PayChan.h index 2e09c473dc..b25a4529be 100644 --- a/src/xrpld/app/tx/detail/PayChan.h +++ b/src/xrpld/app/tx/detail/PayChan.h @@ -36,6 +36,9 @@ public: static TxConsequences makeTxConsequences(PreflightContext const& ctx); + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); @@ -62,6 +65,9 @@ public: static TxConsequences makeTxConsequences(PreflightContext const& ctx); + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); @@ -82,6 +88,12 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/Payment.cpp b/src/xrpld/app/tx/detail/Payment.cpp index a627e3ec53..426b314150 100644 --- a/src/xrpld/app/tx/detail/Payment.cpp +++ b/src/xrpld/app/tx/detail/Payment.cpp @@ -65,20 +65,33 @@ getMaxSourceAmount( dstAmount < beast::zero); } -NotTEC -Payment::preflight(PreflightContext const& ctx) +bool +Payment::checkExtraFeatures(PreflightContext const& ctx) { if (ctx.tx.isFieldPresent(sfCredentialIDs) && !ctx.rules.enabled(featureCredentials)) - return temDISABLED; - + return false; if (ctx.tx.isFieldPresent(sfDomainID) && !ctx.rules.enabled(featurePermissionedDEX)) - return temDISABLED; + return false; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; + return true; +} +std::uint32_t +Payment::getFlagsMask(PreflightContext const& ctx) +{ + auto& tx = ctx.tx; + + STAmount const dstAmount(tx.getFieldAmount(sfAmount)); + bool const mptDirect = dstAmount.holds(); + + return mptDirect ? tfMPTPaymentMask : tfPaymentMask; +} + +NotTEC +Payment::preflight(PreflightContext const& ctx) +{ auto& tx = ctx.tx; auto& j = ctx.j; @@ -90,14 +103,6 @@ Payment::preflight(PreflightContext const& ctx) std::uint32_t const txFlags = tx.getFlags(); - std::uint32_t paymentMask = mptDirect ? tfMPTPaymentMask : tfPaymentMask; - - if (txFlags & paymentMask) - { - JLOG(j.trace()) << "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } - if (txFlags & tfSponsorCreatedAccount) { if (!ctx.rules.enabled(featureSponsor)) @@ -251,7 +256,7 @@ Payment::preflight(PreflightContext const& ctx) !isTesSuccess(err)) return err; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/Payment.h b/src/xrpld/app/tx/detail/Payment.h index 010a2453cf..04bba390e2 100644 --- a/src/xrpld/app/tx/detail/Payment.h +++ b/src/xrpld/app/tx/detail/Payment.h @@ -42,6 +42,12 @@ public: static TxConsequences makeTxConsequences(PreflightContext const& ctx); + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp b/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp index 7c9728a380..f71538b925 100644 --- a/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp +++ b/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp @@ -27,23 +27,11 @@ namespace ripple { NotTEC PermissionedDomainDelete::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featurePermissionedDomains)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - { - JLOG(ctx.j.debug()) << "PermissionedDomainDelete: invalid flags."; - return temINVALID_FLAG; - } - auto const domain = ctx.tx.getFieldH256(sfDomainID); if (domain == beast::zero) return temMALFORMED; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp index 590d5d379d..4eb1d1fcde 100644 --- a/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp +++ b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp @@ -28,22 +28,15 @@ namespace ripple { +bool +PermissionedDomainSet::checkExtraFeatures(PreflightContext const& ctx) +{ + return ctx.rules.enabled(featureCredentials); +} + NotTEC PermissionedDomainSet::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featurePermissionedDomains) || - !ctx.rules.enabled(featureCredentials)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - { - JLOG(ctx.j.debug()) << "PermissionedDomainSet: invalid flags."; - return temINVALID_FLAG; - } - if (auto err = credentials::checkArray( ctx.tx.getFieldArray(sfAcceptedCredentials), maxPermissionedDomainCredentialsArraySize, @@ -55,7 +48,7 @@ PermissionedDomainSet::preflight(PreflightContext const& ctx) if (domain && *domain == beast::zero) return temMALFORMED; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/PermissionedDomainSet.h b/src/xrpld/app/tx/detail/PermissionedDomainSet.h index 502d576e32..ed27896a3b 100644 --- a/src/xrpld/app/tx/detail/PermissionedDomainSet.h +++ b/src/xrpld/app/tx/detail/PermissionedDomainSet.h @@ -33,6 +33,9 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/SetAccount.cpp b/src/xrpld/app/tx/detail/SetAccount.cpp index ebf30e15db..5c581c493a 100644 --- a/src/xrpld/app/tx/detail/SetAccount.cpp +++ b/src/xrpld/app/tx/detail/SetAccount.cpp @@ -57,23 +57,20 @@ SetAccount::makeTxConsequences(PreflightContext const& ctx) return TxConsequences{ctx.tx, getTxConsequencesCategory(ctx.tx)}; } +std::uint32_t +SetAccount::getFlagsMask(PreflightContext const& ctx) +{ + return tfAccountSetMask; +} + NotTEC SetAccount::preflight(PreflightContext const& ctx) { - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - auto& tx = ctx.tx; auto& j = ctx.j; std::uint32_t const uTxFlags = tx.getFlags(); - if (uTxFlags & tfAccountSetMask) - { - JLOG(j.trace()) << "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } - std::uint32_t const uSetFlag = tx.getFieldU32(sfSetFlag); std::uint32_t const uClearFlag = tx.getFieldU32(sfClearFlag); @@ -186,7 +183,7 @@ SetAccount::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/SetAccount.h b/src/xrpld/app/tx/detail/SetAccount.h index ed4242c250..bcc0a61b1b 100644 --- a/src/xrpld/app/tx/detail/SetAccount.h +++ b/src/xrpld/app/tx/detail/SetAccount.h @@ -38,6 +38,9 @@ public: static TxConsequences makeTxConsequences(PreflightContext const& ctx); + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/SetOracle.cpp b/src/xrpld/app/tx/detail/SetOracle.cpp index 61e1022bef..4fd45ea4c4 100644 --- a/src/xrpld/app/tx/detail/SetOracle.cpp +++ b/src/xrpld/app/tx/detail/SetOracle.cpp @@ -39,15 +39,6 @@ tokenPairKey(STObject const& pair) NotTEC SetOracle::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featurePriceOracle)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - auto const& dataSeries = ctx.tx.getFieldArray(sfPriceDataSeries); if (dataSeries.empty()) return temARRAY_EMPTY; @@ -64,7 +55,7 @@ SetOracle::preflight(PreflightContext const& ctx) isInvalidLength(sfAssetClass, maxOracleSymbolClass)) return temMALFORMED; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/SetRegularKey.cpp b/src/xrpld/app/tx/detail/SetRegularKey.cpp index 92d130a15a..4e063e7d1f 100644 --- a/src/xrpld/app/tx/detail/SetRegularKey.cpp +++ b/src/xrpld/app/tx/detail/SetRegularKey.cpp @@ -51,18 +51,6 @@ SetRegularKey::calculateBaseFee(ReadView const& view, STTx const& tx) NotTEC SetRegularKey::preflight(PreflightContext const& ctx) { - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - std::uint32_t const uTxFlags = ctx.tx.getFlags(); - - if (uTxFlags & tfUniversalMask) - { - JLOG(ctx.j.trace()) << "Malformed transaction: Invalid flags set."; - - return temINVALID_FLAG; - } - if (ctx.rules.enabled(fixMasterKeyAsRegularKey) && ctx.tx.isFieldPresent(sfRegularKey) && (ctx.tx.getAccountID(sfRegularKey) == ctx.tx.getAccountID(sfAccount))) @@ -70,7 +58,7 @@ SetRegularKey::preflight(PreflightContext const& ctx) return temBAD_REGKEY; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/SetSignerList.cpp b/src/xrpld/app/tx/detail/SetSignerList.cpp index 7d6c44e178..9af69ae652 100644 --- a/src/xrpld/app/tx/detail/SetSignerList.cpp +++ b/src/xrpld/app/tx/detail/SetSignerList.cpp @@ -77,19 +77,16 @@ SetSignerList::determineOperation( return std::make_tuple(tesSUCCESS, quorum, sign, op); } +std::uint32_t +SetSignerList::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fixInvalidTxFlags) ? tfUniversalMask : 0; +} + NotTEC SetSignerList::preflight(PreflightContext const& ctx) { - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.rules.enabled(fixInvalidTxFlags) && - (ctx.tx.getFlags() & tfUniversalMask)) - { - JLOG(ctx.j.debug()) << "SetSignerList: invalid flags."; - return temINVALID_FLAG; - } - auto const result = determineOperation(ctx.tx, ctx.flags, ctx.j); if (std::get<0>(result) != tesSUCCESS) @@ -119,7 +116,7 @@ SetSignerList::preflight(PreflightContext const& ctx) } } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/SetSignerList.h b/src/xrpld/app/tx/detail/SetSignerList.h index 1827aca975..be2df8152e 100644 --- a/src/xrpld/app/tx/detail/SetSignerList.h +++ b/src/xrpld/app/tx/detail/SetSignerList.h @@ -51,6 +51,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/SetTrust.cpp b/src/xrpld/app/tx/detail/SetTrust.cpp index 7e1e2a77fa..4f7c7e59f3 100644 --- a/src/xrpld/app/tx/detail/SetTrust.cpp +++ b/src/xrpld/app/tx/detail/SetTrust.cpp @@ -67,23 +67,20 @@ computeFreezeFlags( namespace ripple { +std::uint32_t +SetTrust::getFlagsMask(PreflightContext const& ctx) +{ + return tfTrustSetMask; +} + NotTEC SetTrust::preflight(PreflightContext const& ctx) { - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - auto& tx = ctx.tx; auto& j = ctx.j; std::uint32_t const uTxFlags = tx.getFlags(); - if (uTxFlags & tfTrustSetMask) - { - JLOG(j.trace()) << "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } - if (!ctx.rules.enabled(featureDeepFreeze)) { // Even though the deep freeze flags are included in the @@ -127,7 +124,7 @@ SetTrust::preflight(PreflightContext const& ctx) return temDST_NEEDED; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/SetTrust.h b/src/xrpld/app/tx/detail/SetTrust.h index a0476918ac..443080bf74 100644 --- a/src/xrpld/app/tx/detail/SetTrust.h +++ b/src/xrpld/app/tx/detail/SetTrust.h @@ -35,6 +35,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index 2ab5c399f8..e8dc544d27 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -42,7 +42,7 @@ namespace ripple { /** Performs early sanity checks on the txid */ NotTEC -preflight0(PreflightContext const& ctx) +preflight0(PreflightContext const& ctx, std::uint32_t flagMask) { if (isPseudoTx(ctx.tx) && ctx.tx.isFlag(tfInnerBatchTxn)) { @@ -84,12 +84,84 @@ preflight0(PreflightContext const& ctx) return temINVALID; } + if (ctx.tx.getFlags() & flagMask) + { + JLOG(ctx.j.debug()) + << ctx.tx.peekAtField(sfTransactionType).getFullText() + << ": invalid flags."; + return temINVALID_FLAG; + } + return tesSUCCESS; } +namespace detail { + +/** Checks the validity of the transactor signing key. + * + * Normally called from preflight1. + */ +NotTEC +preflightCheckSigningKey(STObject const& sigObject, beast::Journal j) +{ + if (auto const spk = sigObject.getFieldVL(sfSigningPubKey); + !spk.empty() && !publicKeyType(makeSlice(spk))) + { + JLOG(j.debug()) << "preflightCheckSigningKey: invalid signing key"; + return temBAD_SIGNATURE; + } + return tesSUCCESS; +} + +std::optional +preflightCheckSimulateKeys( + ApplyFlags flags, + STObject const& sigObject, + beast::Journal j) +{ + if (flags & tapDRY_RUN) // simulation + { + std::optional const signature = sigObject[~sfTxnSignature]; + if (signature && !signature->empty()) + { + // NOTE: This code should never be hit because it's checked in the + // `simulate` RPC + return temINVALID; // LCOV_EXCL_LINE + } + + if (!sigObject.isFieldPresent(sfSigners)) + { + // no signers, no signature - a valid simulation + return tesSUCCESS; + } + + for (auto const& signer : sigObject.getFieldArray(sfSigners)) + { + if (signer.isFieldPresent(sfTxnSignature) && + !signer[sfTxnSignature].empty()) + { + // NOTE: This code should never be hit because it's + // checked in the `simulate` RPC + return temINVALID; // LCOV_EXCL_LINE + } + } + + Slice const signingPubKey = sigObject[sfSigningPubKey]; + if (!signingPubKey.empty()) + { + // trying to single-sign _and_ multi-sign a transaction + return temINVALID; + } + return tesSUCCESS; + } + return {}; +} + +} // namespace detail + /** Performs early sanity checks on the account and fee fields */ NotTEC -preflight1(PreflightContext const& ctx) +Transactor::preflight1(PreflightContext const& ctx, std::uint32_t flagMask) { // This is inappropriate in preflight0, because only Change transactions // skip this function, and those do not allow an sfTicketSequence field. @@ -113,8 +185,7 @@ preflight1(PreflightContext const& ctx) return temDISABLED; } - auto const ret = preflight0(ctx); - if (!isTesSuccess(ret)) + if (auto const ret = preflight0(ctx, flagMask)) return ret; auto const id = ctx.tx.getAccountID(sfAccount); @@ -132,13 +203,8 @@ preflight1(PreflightContext const& ctx) return temBAD_FEE; } - auto const spk = ctx.tx.getSigningPubKey(); - - if (!spk.empty() && !publicKeyType(makeSlice(spk))) - { - JLOG(ctx.j.debug()) << "preflight1: invalid signing key"; - return temBAD_SIGNATURE; - } + if (auto const ret = detail::preflightCheckSigningKey(ctx.tx, ctx.j)) + return ret; // An AccountTxnID field constrains transaction ordering more than the // Sequence field. Tickets, on the other hand, reduce ordering @@ -179,41 +245,13 @@ preflight1(PreflightContext const& ctx) /** Checks whether the signature appears valid */ NotTEC -preflight2(PreflightContext const& ctx) +Transactor::preflight2(PreflightContext const& ctx) { - if (ctx.flags & tapDRY_RUN) // simulation - { - if (!ctx.tx.getSignature().empty()) - { - // NOTE: This code should never be hit because it's checked in the - // `simulate` RPC - return temINVALID; // LCOV_EXCL_LINE - } - - if (!ctx.tx.isFieldPresent(sfSigners)) - { - // no signers, no signature - a valid simulation - return tesSUCCESS; - } - - for (auto const& signer : ctx.tx.getFieldArray(sfSigners)) - { - if (signer.isFieldPresent(sfTxnSignature) && - !signer[sfTxnSignature].empty()) - { - // NOTE: This code should never be hit because it's - // checked in the `simulate` RPC - return temINVALID; // LCOV_EXCL_LINE - } - } - - if (!ctx.tx.getSigningPubKey().empty()) - { - // trying to single-sign _and_ multi-sign a transaction - return temINVALID; - } - return tesSUCCESS; - } + if (auto const ret = + detail::preflightCheckSimulateKeys(ctx.flags, ctx.tx, ctx.j)) + // Skips following checks if the transaction is being simulated, + // regardless of success or failure + return *ret; auto const sigValid = checkValidity( ctx.app.getHashRouter(), ctx.tx, ctx.rules, ctx.app.config()); @@ -235,6 +273,28 @@ Transactor::Transactor(ApplyContext& ctx) { } +bool +Transactor::validDataLength( + std::optional const& slice, + std::size_t maxLength) +{ + if (!slice) + return true; + return !slice->empty() && slice->length() <= maxLength; +} + +std::uint32_t +Transactor::getFlagsMask(PreflightContext const& ctx) +{ + return tfUniversalMask; +} + +NotTEC +Transactor::preflightSigValidated(PreflightContext const& ctx) +{ + return tesSUCCESS; +} + TER Transactor::checkPermission(ReadView const& view, STTx const& tx) { @@ -315,6 +375,27 @@ Transactor::calculateBaseFee(ReadView const& view, STTx const& tx) return baseFee + ((signerCount + sponsorSignerCount) * baseFee); } +// Returns the fee in fee units, not scaled for load. +XRPAmount +Transactor::calculateOwnerReserveFee(ReadView const& view, STTx const& tx) +{ + // Assumption: One reserve increment is typically much greater than one base + // fee. + // This check is in an assert so that it will come to the attention of + // developers if that assumption is not correct. If the owner reserve is not + // significantly larger than the base fee (or even worse, smaller), we will + // need to rethink charging an owner reserve as a transaction fee. + // TODO: This function is static, and I don't want to add more parameters. + // When it is finally refactored to be in a context that has access to the + // Application, include "app().overlay().networkID() > 2 ||" in the + // condition. + XRPL_ASSERT( + view.fees().increment > view.fees().base * 100, + "ripple::Transactor::calculateOwnerReserveFee : Owner reserve is " + "reasonable"); + return view.fees().increment; +} + XRPAmount Transactor::minimumFee( Application& app, @@ -678,16 +759,19 @@ Transactor::apply() } NotTEC -Transactor::checkSign(PreclaimContext const& ctx) +Transactor::checkSign( + PreclaimContext const& ctx, + AccountID const& idAccount, + STObject const& sigObject) { - auto const pkSigner = ctx.tx.getSigningPubKey(); + auto const pkSigner = sigObject.getFieldVL(sfSigningPubKey); // Ignore signature check on batch inner transactions - if (ctx.tx.isFlag(tfInnerBatchTxn) && + if (sigObject.isFlag(tfInnerBatchTxn) && ctx.view.rules().enabled(featureBatch)) { // Defensive Check: These values are also checked in Batch::preflight - if (ctx.tx.isFieldPresent(sfTxnSignature) || !pkSigner.empty() || - ctx.tx.isFieldPresent(sfSigners)) + if (sigObject.isFieldPresent(sfTxnSignature) || !pkSigner.empty() || + sigObject.isFieldPresent(sfSigners)) { return temINVALID_FLAG; // LCOV_EXCL_LINE } @@ -695,21 +779,18 @@ Transactor::checkSign(PreclaimContext const& ctx) } if ((ctx.flags & tapDRY_RUN) && pkSigner.empty() && - !ctx.tx.isFieldPresent(sfSigners)) + !sigObject.isFieldPresent(sfSigners)) { // simulate: skip signature validation when neither SigningPubKey nor // Signers are provided return tesSUCCESS; } - auto const idAccount = ctx.tx[~sfDelegate].value_or(ctx.tx[sfAccount]); - // If the pk is empty and not simulate or simulate and signers, // then we must be multi-signing. if (ctx.tx.isFieldPresent(sfSigners)) { - STArray const& txSigners(ctx.tx.getFieldArray(sfSigners)); - return checkMultiSign(ctx.view, idAccount, txSigners, ctx.flags, ctx.j); + return checkMultiSign(ctx, idAccount, sigObject); } if (ctx.tx.isFieldPresent(sfSponsor)) @@ -727,15 +808,15 @@ Transactor::checkSign(PreclaimContext const& ctx) // Check Single Sign XRPL_ASSERT( - !pkSigner.empty(), - "ripple::Transactor::checkSingleSign : non-empty signer or simulation"); + !pkSigner.empty(), "ripple::Transactor::checkSign : non-empty signer"); if (!publicKeyType(makeSlice(pkSigner))) { - JLOG(ctx.j.trace()) - << "checkSingleSign: signing public key type is unknown"; + JLOG(ctx.j.trace()) << "checkSign: signing public key type is unknown"; return tefBAD_AUTH; // FIXME: should be better error! } + + // Look up the account. auto const idSigner = pkSigner.empty() ? idAccount : calcAccountID(PublicKey(makeSlice(pkSigner))); @@ -743,8 +824,16 @@ Transactor::checkSign(PreclaimContext const& ctx) if (!sleAccount) return terNO_ACCOUNT; - return checkSingleSign( - idSigner, idAccount, sleAccount, ctx.view.rules(), ctx.j); + return checkSingleSign(ctx, idSigner, idAccount, sleAccount); +} + +NotTEC +Transactor::checkSign(PreclaimContext const& ctx) +{ + auto const idAccount = ctx.tx.isFieldPresent(sfDelegate) + ? ctx.tx.getAccountID(sfDelegate) + : ctx.tx.getAccountID(sfAccount); + return checkSign(ctx, idAccount, ctx.tx); } NotTEC @@ -759,9 +848,7 @@ Transactor::checkBatchSign(PreclaimContext const& ctx) Blob const& pkSigner = signer.getFieldVL(sfSigningPubKey); if (pkSigner.empty()) { - STArray const& txSigners(signer.getFieldArray(sfSigners)); - if (ret = checkMultiSign( - ctx.view, idAccount, txSigners, ctx.flags, ctx.j); + if (ret = checkMultiSign(ctx, idAccount, signer); !isTesSuccess(ret)) return ret; } @@ -785,8 +872,7 @@ Transactor::checkBatchSign(PreclaimContext const& ctx) return tesSUCCESS; } - if (ret = checkSingleSign( - idSigner, idAccount, sleAccount, ctx.view.rules(), ctx.j); + if (ret = checkSingleSign(ctx, idSigner, idAccount, sleAccount); !isTesSuccess(ret)) return ret; } @@ -814,10 +900,12 @@ Transactor::checkSponsorSign(PreclaimContext const& ctx) if (pkSigner.empty()) { STArray const& txSigners(sponsorObj.getFieldArray(sfSigners)); - if (ret = checkMultiSign( - ctx.view, sponsorAcc, txSigners, ctx.flags, ctx.j); - !isTesSuccess(ret)) - return ret; + for (auto const& txSigner : txSigners) + { + if (ret = checkMultiSign(ctx, sponsorAcc, txSigner); + !isTesSuccess(ret)) + return ret; + } } else { @@ -828,8 +916,7 @@ Transactor::checkSponsorSign(PreclaimContext const& ctx) auto const idSigner = calcAccountID(PublicKey(makeSlice(pkSigner))); - if (ret = checkSingleSign( - idSigner, sponsorAcc, sleAccount, ctx.view.rules(), ctx.j); + if (ret = checkSingleSign(ctx, idSigner, sponsorAcc, sleAccount); !isTesSuccess(ret)) { return ret; @@ -841,15 +928,14 @@ Transactor::checkSponsorSign(PreclaimContext const& ctx) NotTEC Transactor::checkSingleSign( + PreclaimContext const& ctx, AccountID const& idSigner, AccountID const& idAccount, - std::shared_ptr sleAccount, - Rules const& rules, - beast::Journal j) + std::shared_ptr sleAccount) { bool const isMasterDisabled = sleAccount->isFlag(lsfDisableMaster); - if (rules.enabled(fixMasterKeyAsRegularKey)) + if (ctx.view.rules().enabled(fixMasterKeyAsRegularKey)) { // Signed with regular key. if ((*sleAccount)[~sfRegularKey] == idSigner) @@ -886,14 +972,16 @@ Transactor::checkSingleSign( else if (sleAccount->isFieldPresent(sfRegularKey)) { // Signing key does not match master or regular key. - JLOG(j.trace()) << "checkSingleSign: Not authorized to use account."; + JLOG(ctx.j.trace()) + << "checkSingleSign: Not authorized to use account."; return tefBAD_AUTH; } else { // No regular key on account and signing key does not match master key. // FIXME: Why differentiate this case from tefBAD_AUTH? - JLOG(j.trace()) << "checkSingleSign: Not authorized to use account."; + JLOG(ctx.j.trace()) + << "checkSingleSign: Not authorized to use account."; return tefBAD_AUTH_MASTER; } @@ -902,19 +990,17 @@ Transactor::checkSingleSign( NotTEC Transactor::checkMultiSign( - ReadView const& view, + PreclaimContext const& ctx, AccountID const& id, - STArray const& txSigners, - ApplyFlags const& flags, - beast::Journal j) + STObject const& sigObject) { - // Get mTxnAccountID's SignerList and Quorum. + // Get id's SignerList and Quorum. std::shared_ptr sleAccountSigners = - view.read(keylet::signers(id)); + ctx.view.read(keylet::signers(id)); // If the signer list doesn't exist the account is not multi-signing. if (!sleAccountSigners) { - JLOG(j.trace()) + JLOG(ctx.j.trace()) << "applyTransaction: Invalid: Not a multi-signing account."; return tefNOT_MULTI_SIGNING; } @@ -929,11 +1015,12 @@ Transactor::checkMultiSign( "ripple::Transactor::checkMultiSign : signer list ID is 0"); auto accountSigners = - SignerEntries::deserialize(*sleAccountSigners, j, "ledger"); + SignerEntries::deserialize(*sleAccountSigners, ctx.j, "ledger"); if (!accountSigners) return accountSigners.error(); // Get the array of transaction signers. + STArray const& txSigners(sigObject.getFieldArray(sfSigners)); // Walk the accountSigners performing a variety of checks and see if // the quorum is met. @@ -952,7 +1039,7 @@ Transactor::checkMultiSign( { if (++iter == accountSigners->end()) { - JLOG(j.trace()) + JLOG(ctx.j.trace()) << "applyTransaction: Invalid SigningAccount.Account."; return tefBAD_SIGNATURE; } @@ -960,7 +1047,7 @@ Transactor::checkMultiSign( if (iter->account != txSignerAcctID) { // The SigningAccount is not in the SignerEntries. - JLOG(j.trace()) + JLOG(ctx.j.trace()) << "applyTransaction: Invalid SigningAccount.Account."; return tefBAD_SIGNATURE; } @@ -974,13 +1061,13 @@ Transactor::checkMultiSign( // STTx::checkMultiSign if (!spk.empty() && !publicKeyType(makeSlice(spk))) { - JLOG(j.trace()) + JLOG(ctx.j.trace()) << "checkMultiSign: signing public key type is unknown"; return tefBAD_SIGNATURE; } XRPL_ASSERT( - (flags & tapDRY_RUN) || !spk.empty(), + (ctx.flags & tapDRY_RUN) || !spk.empty(), "ripple::Transactor::checkMultiSign : non-empty signer or " "simulation"); AccountID const signingAcctIDFromPubKey = spk.empty() @@ -1012,7 +1099,8 @@ Transactor::checkMultiSign( // In any of these cases we need to know whether the account is in // the ledger. Determine that now. - auto const sleTxSignerRoot = view.read(keylet::account(txSignerAcctID)); + auto const sleTxSignerRoot = + ctx.view.read(keylet::account(txSignerAcctID)); if (signingAcctIDFromPubKey == txSignerAcctID) { @@ -1025,7 +1113,7 @@ Transactor::checkMultiSign( if (signerAccountFlags & lsfDisableMaster) { - JLOG(j.trace()) + JLOG(ctx.j.trace()) << "applyTransaction: Signer:Account lsfDisableMaster."; return tefMASTER_DISABLED; } @@ -1037,21 +1125,21 @@ Transactor::checkMultiSign( // Public key must hash to the account's regular key. if (!sleTxSignerRoot) { - JLOG(j.trace()) << "applyTransaction: Non-phantom signer " - "lacks account root."; + JLOG(ctx.j.trace()) << "applyTransaction: Non-phantom signer " + "lacks account root."; return tefBAD_SIGNATURE; } if (!sleTxSignerRoot->isFieldPresent(sfRegularKey)) { - JLOG(j.trace()) + JLOG(ctx.j.trace()) << "applyTransaction: Account lacks RegularKey."; return tefBAD_SIGNATURE; } if (signingAcctIDFromPubKey != sleTxSignerRoot->getAccountID(sfRegularKey)) { - JLOG(j.trace()) + JLOG(ctx.j.trace()) << "applyTransaction: Account doesn't match RegularKey."; return tefBAD_SIGNATURE; } @@ -1063,7 +1151,8 @@ Transactor::checkMultiSign( // Cannot perform transaction if quorum is not met. if (weightSum < sleAccountSigners->getFieldU32(sfSignerQuorum)) { - JLOG(j.trace()) << "applyTransaction: Signers failed to meet quorum."; + JLOG(ctx.j.trace()) + << "applyTransaction: Signers failed to meet quorum."; return tefBAD_QUORUM; } diff --git a/src/xrpld/app/tx/detail/Transactor.h b/src/xrpld/app/tx/detail/Transactor.h index fb7eb9159c..4d79f3b721 100644 --- a/src/xrpld/app/tx/detail/Transactor.h +++ b/src/xrpld/app/tx/detail/Transactor.h @@ -134,6 +134,8 @@ public: class TxConsequences; struct PreflightResult; +// Needed for preflight specialization +class Change; enum class FeePayerType { Account, @@ -215,6 +217,35 @@ public: static XRPAmount calculateBaseFee(ReadView const& view, STTx const& tx); + /* Do NOT define an invokePreflight function in a derived class. + Instead, define: + + // Optional if the transaction is gated on an amendment that + // isn't specified in transactions.macro + static bool + checkExtraFeatures(PreflightContext const& ctx); + + // Optional if the transaction uses any flags other than tfUniversal + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + + // Required, even if it just returns tesSUCCESS. + static NotTEC + preflight(PreflightContext const& ctx); + + // Optional, rarely needed, if the transaction does any expensive + // checks after the signature is verified. + static NotTEC preflightSigValidated(PreflightContext const& ctx); + + * Do not try to call preflight1 or preflight2 directly. + * Do not check whether relevant amendments are enabled in preflight. + Instead, define checkExtraFeatures. + * Do not check flags in preflight. Instead, define getFlagsMask. + */ + template + static NotTEC + invokePreflight(PreflightContext const& ctx); + static TER preclaim(PreclaimContext const& ctx) { @@ -266,6 +297,42 @@ protected: Fees const& fees, ApplyFlags flags); + // Returns the fee in fee units, not scaled for load. + static XRPAmount + calculateOwnerReserveFee(ReadView const& view, STTx const& tx); + + static NotTEC + checkSign( + PreclaimContext const& ctx, + AccountID const& id, + STObject const& sigObject); + + // Base class always returns true + static bool + checkExtraFeatures(PreflightContext const& ctx); + + // Base class always returns tfUniversalMask + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + + // Base class always returns tesSUCCESS + static NotTEC + preflightSigValidated(PreflightContext const& ctx); + + static bool + validDataLength(std::optional const& slice, std::size_t maxLength); + + template + static bool + validNumericRange(std::optional value, T max, T min = {}); + + template + static bool + validNumericRange( + std::optional value, + unit::ValueUnit max, + unit::ValueUnit min = {}); + private: std::pair reset(XRPAmount fee); @@ -279,33 +346,117 @@ private: payFee(); static NotTEC checkSingleSign( + PreclaimContext const& ctx, AccountID const& idSigner, AccountID const& idAccount, - std::shared_ptr sleAccount, - Rules const& rules, - beast::Journal j); + std::shared_ptr sleAccount); static NotTEC checkMultiSign( - ReadView const& view, - AccountID const& idAccount, - STArray const& txSigners, - ApplyFlags const& flags, - beast::Journal j); + PreclaimContext const& ctx, + AccountID const& id, + STObject const& sigObject); void trapTransaction(uint256) const; + + /** Performs early sanity checks on the account and fee fields. + + (And passes flagMask to preflight0) + + Do not try to call preflight1 from preflight() in derived classes. See + the description of invokePreflight for details. + */ + static NotTEC + preflight1(PreflightContext const& ctx, std::uint32_t flagMask); + + /** Checks whether the signature appears valid + + Do not try to call preflight2 from preflight() in derived classes. See + the description of invokePreflight for details. + */ + static NotTEC + preflight2(PreflightContext const& ctx); }; -/** Performs early sanity checks on the txid */ -NotTEC -preflight0(PreflightContext const& ctx); +inline bool +Transactor::checkExtraFeatures(PreflightContext const& ctx) +{ + return true; +} -/** Performs early sanity checks on the account and fee fields */ +/** Performs early sanity checks on the txid and flags */ NotTEC -preflight1(PreflightContext const& ctx); +preflight0(PreflightContext const& ctx, std::uint32_t flagMask); -/** Checks whether the signature appears valid */ +namespace detail { + +/** Checks the validity of the transactor signing key. + * + * Normally called from preflight1 with ctx.tx. + */ NotTEC -preflight2(PreflightContext const& ctx); +preflightCheckSigningKey(STObject const& sigObject, beast::Journal j); + +/** Checks the special signing key state needed for simulation + * + * Normally called from preflight2 with ctx.tx. + */ +std::optional +preflightCheckSimulateKeys( + ApplyFlags flags, + STObject const& sigObject, + beast::Journal j); +} // namespace detail + +// Defined in Change.cpp +template <> +NotTEC +Transactor::invokePreflight(PreflightContext const& ctx); + +template +NotTEC +Transactor::invokePreflight(PreflightContext const& ctx) +{ + // Using this lookup does NOT require checking the fixDelegateV1_1. The data + // exists regardless of whether it is enabled. + auto const feature = + Permission::getInstance().getTxFeature(ctx.tx.getTxnType()); + + if (feature && !ctx.rules.enabled(*feature)) + return temDISABLED; + + if (!T::checkExtraFeatures(ctx)) + return temDISABLED; + + if (auto const ret = preflight1(ctx, T::getFlagsMask(ctx))) + return ret; + + if (auto const ret = T::preflight(ctx)) + return ret; + + if (auto const ret = preflight2(ctx)) + return ret; + + return T::preflightSigValidated(ctx); +} + +template +bool +Transactor::validNumericRange(std::optional value, T max, T min) +{ + if (!value) + return true; + return value >= min && value <= max; +} + +template +bool +Transactor::validNumericRange( + std::optional value, + unit::ValueUnit max, + unit::ValueUnit min) +{ + return validNumericRange(value, max.value(), min.value()); +} } // namespace ripple diff --git a/src/xrpld/app/tx/detail/VaultClawback.cpp b/src/xrpld/app/tx/detail/VaultClawback.cpp index 2d09173ac2..6d42f5b597 100644 --- a/src/xrpld/app/tx/detail/VaultClawback.cpp +++ b/src/xrpld/app/tx/detail/VaultClawback.cpp @@ -35,15 +35,6 @@ namespace ripple { NotTEC VaultClawback::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureSingleAssetVault)) - return temDISABLED; - - if (auto const ter = preflight1(ctx)) - return ter; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (ctx.tx[sfVaultID] == beast::zero) { JLOG(ctx.j.debug()) << "VaultClawback: zero/empty vault ID."; @@ -78,7 +69,7 @@ VaultClawback::preflight(PreflightContext const& ctx) } } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/VaultCreate.cpp b/src/xrpld/app/tx/detail/VaultCreate.cpp index 206f6b17d6..4e499e5772 100644 --- a/src/xrpld/app/tx/detail/VaultCreate.cpp +++ b/src/xrpld/app/tx/detail/VaultCreate.cpp @@ -35,28 +35,27 @@ namespace ripple { +bool +VaultCreate::checkExtraFeatures(PreflightContext const& ctx) +{ + if (!ctx.rules.enabled(featureMPTokensV1)) + return false; + + return !ctx.tx.isFieldPresent(sfDomainID) || + ctx.rules.enabled(featurePermissionedDomains); +} + +std::uint32_t +VaultCreate::getFlagsMask(PreflightContext const& ctx) +{ + return tfVaultCreateMask; +} + NotTEC VaultCreate::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureSingleAssetVault) || - !ctx.rules.enabled(featureMPTokensV1)) - return temDISABLED; - - if (ctx.tx.isFieldPresent(sfDomainID) && - !ctx.rules.enabled(featurePermissionedDomains)) - return temDISABLED; - - if (auto const ter = preflight1(ctx)) - return ter; - - if (ctx.tx.getFlags() & tfVaultCreateMask) - return temINVALID_FLAG; - - if (auto const data = ctx.tx[~sfData]) - { - if (data->empty() || data->length() > maxDataPayloadLength) - return temMALFORMED; - } + if (!validDataLength(ctx.tx[~sfData], maxDataPayloadLength)) + return temMALFORMED; if (auto const withdrawalPolicy = ctx.tx[~sfWithdrawalPolicy]) { @@ -96,14 +95,14 @@ VaultCreate::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } XRPAmount VaultCreate::calculateBaseFee(ReadView const& view, STTx const& tx) { // One reserve increment is typically much greater than one base fee. - return view.fees().increment; + return calculateOwnerReserveFee(view, tx); } TER @@ -112,32 +111,8 @@ VaultCreate::preclaim(PreclaimContext const& ctx) auto const vaultAsset = ctx.tx[sfAsset]; auto const account = ctx.tx[sfAccount]; - if (vaultAsset.native()) - ; // No special checks for XRP - else if (vaultAsset.holds()) - { - auto mptID = vaultAsset.get().getMptID(); - auto issuance = ctx.view.read(keylet::mptIssuance(mptID)); - if (!issuance) - return tecOBJECT_NOT_FOUND; - if (!issuance->isFlag(lsfMPTCanTransfer)) - { - // NOTE: flag lsfMPTCanTransfer is immutable, so this is debug in - // VaultCreate only; in other vault function it's an error. - JLOG(ctx.j.debug()) - << "VaultCreate: vault assets are non-transferable."; - return tecNO_AUTH; - } - } - else if (vaultAsset.holds()) - { - auto const issuer = - ctx.view.read(keylet::account(vaultAsset.getIssuer())); - if (!issuer) - return terNO_ACCOUNT; - else if (!issuer->isFlag(lsfDefaultRipple)) - return terNO_RIPPLE; - } + if (auto const ter = canAddHolding(ctx.view, vaultAsset)) + return ter; // Check for pseudo-account issuers - we do not want a vault to hold such // assets (e.g. MPT shares to other vaults or AMM LPTokens) as they would be diff --git a/src/xrpld/app/tx/detail/VaultCreate.h b/src/xrpld/app/tx/detail/VaultCreate.h index 5555644629..3f952d540a 100644 --- a/src/xrpld/app/tx/detail/VaultCreate.h +++ b/src/xrpld/app/tx/detail/VaultCreate.h @@ -33,6 +33,12 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/VaultDelete.cpp b/src/xrpld/app/tx/detail/VaultDelete.cpp index 50d8ebacbf..656c371065 100644 --- a/src/xrpld/app/tx/detail/VaultDelete.cpp +++ b/src/xrpld/app/tx/detail/VaultDelete.cpp @@ -31,22 +31,13 @@ namespace ripple { NotTEC VaultDelete::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureSingleAssetVault)) - return temDISABLED; - - if (auto const ter = preflight1(ctx)) - return ter; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (ctx.tx[sfVaultID] == beast::zero) { JLOG(ctx.j.debug()) << "VaultDelete: zero/empty vault ID."; return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/VaultDeposit.cpp b/src/xrpld/app/tx/detail/VaultDeposit.cpp index 1daaf27efd..ccb7a8dc97 100644 --- a/src/xrpld/app/tx/detail/VaultDeposit.cpp +++ b/src/xrpld/app/tx/detail/VaultDeposit.cpp @@ -36,15 +36,6 @@ namespace ripple { NotTEC VaultDeposit::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureSingleAssetVault)) - return temDISABLED; - - if (auto const ter = preflight1(ctx)) - return ter; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (ctx.tx[sfVaultID] == beast::zero) { JLOG(ctx.j.debug()) << "VaultDeposit: zero/empty vault ID."; @@ -54,7 +45,7 @@ VaultDeposit::preflight(PreflightContext const& ctx) if (ctx.tx[sfAmount] <= beast::zero) return temBAD_AMOUNT; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/VaultSet.cpp b/src/xrpld/app/tx/detail/VaultSet.cpp index 5a519f81cf..6057e40cfa 100644 --- a/src/xrpld/app/tx/detail/VaultSet.cpp +++ b/src/xrpld/app/tx/detail/VaultSet.cpp @@ -30,28 +30,22 @@ namespace ripple { +bool +VaultSet::checkExtraFeatures(PreflightContext const& ctx) +{ + return !ctx.tx.isFieldPresent(sfDomainID) || + ctx.rules.enabled(featurePermissionedDomains); +} + NotTEC VaultSet::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureSingleAssetVault)) - return temDISABLED; - - if (ctx.tx.isFieldPresent(sfDomainID) && - !ctx.rules.enabled(featurePermissionedDomains)) - return temDISABLED; - - if (auto const ter = preflight1(ctx)) - return ter; - if (ctx.tx[sfVaultID] == beast::zero) { JLOG(ctx.j.debug()) << "VaultSet: zero/empty vault ID."; return temMALFORMED; } - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (auto const data = ctx.tx[~sfData]) { if (data->empty() || data->length() > maxDataPayloadLength) @@ -78,7 +72,7 @@ VaultSet::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/VaultSet.h b/src/xrpld/app/tx/detail/VaultSet.h index f16aa6c284..b3eecbbab5 100644 --- a/src/xrpld/app/tx/detail/VaultSet.h +++ b/src/xrpld/app/tx/detail/VaultSet.h @@ -33,6 +33,9 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/VaultWithdraw.cpp b/src/xrpld/app/tx/detail/VaultWithdraw.cpp index 2216df5420..365ab437b0 100644 --- a/src/xrpld/app/tx/detail/VaultWithdraw.cpp +++ b/src/xrpld/app/tx/detail/VaultWithdraw.cpp @@ -33,15 +33,6 @@ namespace ripple { NotTEC VaultWithdraw::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureSingleAssetVault)) - return temDISABLED; - - if (auto const ter = preflight1(ctx)) - return ter; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (ctx.tx[sfVaultID] == beast::zero) { JLOG(ctx.j.debug()) << "VaultWithdraw: zero/empty vault ID."; @@ -68,7 +59,7 @@ VaultWithdraw::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/XChainBridge.cpp b/src/xrpld/app/tx/detail/XChainBridge.cpp index bf5633900c..a7ff41aa3e 100644 --- a/src/xrpld/app/tx/detail/XChainBridge.cpp +++ b/src/xrpld/app/tx/detail/XChainBridge.cpp @@ -1227,17 +1227,8 @@ toClaim(STTx const& tx) template NotTEC -attestationPreflight(PreflightContext const& ctx) +attestationpreflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureXChainBridge)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (!publicKeyType(ctx.tx[sfPublicKey])) return temMALFORMED; @@ -1258,7 +1249,7 @@ attestationPreflight(PreflightContext const& ctx) if (att->sendingAmount.issue() != expectedIssue) return temXCHAIN_BAD_PROOF; - return preflight2(ctx); + return tesSUCCESS; } template @@ -1397,15 +1388,6 @@ attestationDoApply(ApplyContext& ctx) NotTEC XChainCreateBridge::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureXChainBridge)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - auto const account = ctx.tx[sfAccount]; auto const reward = ctx.tx[sfSignatureReward]; auto const minAccountCreate = ctx.tx[~sfMinAccountCreateAmount]; @@ -1475,7 +1457,7 @@ XChainCreateBridge::preflight(PreflightContext const& ctx) return temXCHAIN_BRIDGE_BAD_ISSUES; } - return preflight2(ctx); + return tesSUCCESS; } TER @@ -1577,18 +1559,15 @@ XChainCreateBridge::doApply() //------------------------------------------------------------------------------ +std::uint32_t +BridgeModify::getFlagsMask(PreflightContext const& ctx) +{ + return tfBridgeModifyMask; +} + NotTEC BridgeModify::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureXChainBridge)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfBridgeModifyMask) - return temINVALID_FLAG; - auto const account = ctx.tx[sfAccount]; auto const reward = ctx.tx[~sfSignatureReward]; auto const minAccountCreate = ctx.tx[~sfMinAccountCreateAmount]; @@ -1627,7 +1606,7 @@ BridgeModify::preflight(PreflightContext const& ctx) return temXCHAIN_BRIDGE_BAD_MIN_ACCOUNT_CREATE_AMOUNT; } - return preflight2(ctx); + return tesSUCCESS; } TER @@ -1690,15 +1669,6 @@ BridgeModify::doApply() NotTEC XChainClaim::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureXChainBridge)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - STXChainBridge const bridgeSpec = ctx.tx[sfXChainBridge]; auto const amount = ctx.tx[sfAmount]; @@ -1709,7 +1679,7 @@ XChainClaim::preflight(PreflightContext const& ctx) return temBAD_AMOUNT; } - return preflight2(ctx); + return tesSUCCESS; } TER @@ -1928,15 +1898,6 @@ XChainCommit::makeTxConsequences(PreflightContext const& ctx) NotTEC XChainCommit::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureXChainBridge)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - auto const amount = ctx.tx[sfAmount]; auto const bridgeSpec = ctx.tx[sfXChainBridge]; @@ -1947,7 +1908,7 @@ XChainCommit::preflight(PreflightContext const& ctx) amount.issue() != bridgeSpec.issuingChainIssue()) return temBAD_ISSUER; - return preflight2(ctx); + return tesSUCCESS; } TER @@ -2042,21 +2003,12 @@ XChainCommit::doApply() NotTEC XChainCreateClaimID::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureXChainBridge)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - auto const reward = ctx.tx[sfSignatureReward]; if (!isXRP(reward) || reward.signum() < 0 || !isLegalNet(reward)) return temXCHAIN_BRIDGE_BAD_REWARD_AMOUNT; - return preflight2(ctx); + return tesSUCCESS; } TER @@ -2159,7 +2111,7 @@ XChainCreateClaimID::doApply() NotTEC XChainAddClaimAttestation::preflight(PreflightContext const& ctx) { - return attestationPreflight(ctx); + return attestationpreflight(ctx); } TER @@ -2179,7 +2131,7 @@ XChainAddClaimAttestation::doApply() NotTEC XChainAddAccountCreateAttestation::preflight(PreflightContext const& ctx) { - return attestationPreflight(ctx); + return attestationpreflight(ctx); } TER @@ -2199,15 +2151,6 @@ XChainAddAccountCreateAttestation::doApply() NotTEC XChainCreateAccountCommit::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureXChainBridge)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - auto const amount = ctx.tx[sfAmount]; if (amount.signum() <= 0 || !amount.native()) @@ -2220,7 +2163,7 @@ XChainCreateAccountCommit::preflight(PreflightContext const& ctx) if (reward.issue() != amount.issue()) return temBAD_AMOUNT; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/XChainBridge.h b/src/xrpld/app/tx/detail/XChainBridge.h index 82b64cc0e3..0e9c0358d2 100644 --- a/src/xrpld/app/tx/detail/XChainBridge.h +++ b/src/xrpld/app/tx/detail/XChainBridge.h @@ -58,6 +58,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/applySteps.cpp b/src/xrpld/app/tx/detail/applySteps.cpp index f65f60554a..179a0e5d82 100644 --- a/src/xrpld/app/tx/detail/applySteps.cpp +++ b/src/xrpld/app/tx/detail/applySteps.cpp @@ -119,7 +119,7 @@ invoke_preflight(PreflightContext const& ctx) try { return with_txn_type(ctx.tx.getTxnType(), [&]() { - auto const tec = T::preflight(ctx); + auto const tec = Transactor::invokePreflight(ctx); return std::make_pair( tec, isTesSuccess(tec) ? consequences_helper(ctx)