diff --git a/include/xrpl/protocol/PublicKey.h b/include/xrpl/protocol/PublicKey.h index 445e0e3b97..8d8062408b 100644 --- a/include/xrpl/protocol/PublicKey.h +++ b/include/xrpl/protocol/PublicKey.h @@ -231,11 +231,7 @@ verifyDigest( SHA512-Half, and the resulting digest is signed. */ [[nodiscard]] bool -verify( - PublicKey const& publicKey, - Slice const& m, - Slice const& sig, - bool mustBeFullyCanonical = true) noexcept; +verify(PublicKey const& publicKey, Slice const& m, Slice const& sig) noexcept; /** Calculate the 160-bit node ID from a node public key. */ NodeID diff --git a/include/xrpl/protocol/STTx.h b/include/xrpl/protocol/STTx.h index 716881d910..02f865e349 100644 --- a/include/xrpl/protocol/STTx.h +++ b/include/xrpl/protocol/STTx.h @@ -103,22 +103,15 @@ public: std::optional> signatureTarget = {}); - enum class RequireFullyCanonicalSig : bool { no, yes }; - /** Check the signature. - @param requireCanonicalSig If `true`, check that the signature is fully - canonical. If `false`, only check that the signature is valid. @param rules The current ledger rules. @return `true` if valid signature. If invalid, the error message string. */ Expected - checkSign(RequireFullyCanonicalSig requireCanonicalSig, Rules const& rules) - const; + checkSign(Rules const& rules) const; Expected - checkBatchSign( - RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules) const; + checkBatchSign(Rules const& rules) const; // SQL Functions with metadata. static std::string const& @@ -140,40 +133,25 @@ public: private: /** Check the signature. - @param requireCanonicalSig If `true`, check that the signature is fully - canonical. If `false`, only check that the signature is valid. @param rules The current ledger rules. @param sigObject Reference to object that contains the signature fields. Will be *this more often than not. @return `true` if valid signature. If invalid, the error message string. */ Expected - checkSign( - RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules, - STObject const& sigObject) const; + checkSign(Rules const& rules, STObject const& sigObject) const; Expected - checkSingleSign( - RequireFullyCanonicalSig requireCanonicalSig, - STObject const& sigObject) const; + checkSingleSign(STObject const& sigObject) const; Expected - checkMultiSign( - RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules, - STObject const& sigObject) const; + checkMultiSign(Rules const& rules, STObject const& sigObject) const; Expected - checkBatchSingleSign( - STObject const& batchSigner, - RequireFullyCanonicalSig requireCanonicalSig) const; + checkBatchSingleSign(STObject const& batchSigner) const; Expected - checkBatchMultiSign( - STObject const& batchSigner, - RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules) const; + checkBatchMultiSign(STObject const& batchSigner, Rules const& rules) const; STBase* copy(std::size_t n, void* buf) const override; diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index e5ecac614e..18f2294279 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -68,8 +68,6 @@ XRPL_FIX (UniversalNumber, Supported::yes, VoteBehavior::DefaultNo XRPL_FEATURE(XRPFees, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (RemoveNFTokenAutoTrustLine, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(FlowSortStrands, Supported::yes, VoteBehavior::DefaultYes) -XRPL_FEATURE(RequireFullyCanonicalSig, Supported::yes, VoteBehavior::DefaultYes) -XRPL_FEATURE(DeletableAccounts, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(Flow, Supported::yes, VoteBehavior::DefaultYes) // The following amendments are obsolete, but must remain supported @@ -82,9 +80,9 @@ XRPL_FEATURE(Flow, Supported::yes, VoteBehavior::DefaultYe // enabled (added to the ledger). // // If a feature remains obsolete for long enough that no clients are able -// to vote for it, the feature can be removed (entirely?) from the code. -XRPL_FEATURE(CryptoConditionsSuite, Supported::yes, VoteBehavior::Obsolete) - +// to vote for it, the feature can be removed entirely from the code. Until +// then the feature needs to be marked explicitly as obsolete, e.g. +// XRPL_FEATURE(Example, Supported::yes, VoteBehavior::Obsolete) // The following amendments have been active for at least two years. Their // pre-amendment code has been removed and the identifiers are deprecated. // All known amendments and amendments that may appear in a validated ledger @@ -120,6 +118,8 @@ XRPL_RETIRE_FIX(TrustLinesToSelf) XRPL_RETIRE_FEATURE(Checks) XRPL_RETIRE_FEATURE(CheckCashMakesTrustLine) XRPL_RETIRE_FEATURE(CryptoConditions) +XRPL_RETIRE_FEATURE(CryptoConditionsSuite) +XRPL_RETIRE_FEATURE(DeletableAccounts) XRPL_RETIRE_FEATURE(DepositAuth) XRPL_RETIRE_FEATURE(DepositPreauth) XRPL_RETIRE_FEATURE(DisallowIncoming) @@ -135,6 +135,7 @@ XRPL_RETIRE_FEATURE(MultiSignReserve) XRPL_RETIRE_FEATURE(NegativeUNL) XRPL_RETIRE_FEATURE(NonFungibleTokensV1_1) XRPL_RETIRE_FEATURE(PayChan) +XRPL_RETIRE_FEATURE(RequireFullyCanonicalSig) XRPL_RETIRE_FEATURE(SortedDirectories) XRPL_RETIRE_FEATURE(TicketBatch) XRPL_RETIRE_FEATURE(TickSize) diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index dd1b52b355..6d2d833440 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -297,7 +297,7 @@ TRANSACTION(ttTRUST_SET, 20, TrustSet, #endif TRANSACTION(ttACCOUNT_DELETE, 21, AccountDelete, Delegation::notDelegatable, - featureDeletableAccounts, + uint256{}, mustDeleteAcct, ({ {sfDestination, soeREQUIRED}, diff --git a/src/libxrpl/ledger/View.cpp b/src/libxrpl/ledger/View.cpp index a91a0c6468..33c6cf69ec 100644 --- a/src/libxrpl/ledger/View.cpp +++ b/src/libxrpl/ledger/View.cpp @@ -2138,7 +2138,7 @@ rippleSendIOU( // Direct send: redeeming IOUs and/or sending own IOUs. auto const ter = rippleCreditIOU(view, uSenderID, uReceiverID, saAmount, false, j); - if (view.rules().enabled(featureDeletableAccounts) && ter != tesSUCCESS) + if (ter != tesSUCCESS) return ter; saActual = saAmount; return tesSUCCESS; diff --git a/src/libxrpl/protocol/PublicKey.cpp b/src/libxrpl/protocol/PublicKey.cpp index 0a29af7d32..f1794331ad 100644 --- a/src/libxrpl/protocol/PublicKey.cpp +++ b/src/libxrpl/protocol/PublicKey.cpp @@ -267,18 +267,13 @@ verifyDigest( } bool -verify( - PublicKey const& publicKey, - Slice const& m, - Slice const& sig, - bool mustBeFullyCanonical) noexcept +verify(PublicKey const& publicKey, Slice const& m, Slice const& sig) noexcept { if (auto const type = publicKeyType(publicKey)) { if (*type == KeyType::secp256k1) { - return verifyDigest( - publicKey, sha512Half(m), sig, mustBeFullyCanonical); + return verifyDigest(publicKey, sha512Half(m), sig); } else if (*type == KeyType::ed25519) { diff --git a/src/libxrpl/protocol/STTx.cpp b/src/libxrpl/protocol/STTx.cpp index 006f4fee7a..a6f134352e 100644 --- a/src/libxrpl/protocol/STTx.cpp +++ b/src/libxrpl/protocol/STTx.cpp @@ -237,10 +237,7 @@ STTx::sign( } Expected -STTx::checkSign( - RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules, - STObject const& sigObject) const +STTx::checkSign(Rules const& rules, STObject const& sigObject) const { try { @@ -249,9 +246,8 @@ STTx::checkSign( // multi-signing. Otherwise we're single-signing. Blob const& signingPubKey = sigObject.getFieldVL(sfSigningPubKey); - return signingPubKey.empty() - ? checkMultiSign(requireCanonicalSig, rules, sigObject) - : checkSingleSign(requireCanonicalSig, sigObject); + return signingPubKey.empty() ? checkMultiSign(rules, sigObject) + : checkSingleSign(sigObject); } catch (std::exception const&) { @@ -260,17 +256,15 @@ STTx::checkSign( } Expected -STTx::checkSign( - RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules) const +STTx::checkSign(Rules const& rules) const { - if (auto const ret = checkSign(requireCanonicalSig, rules, *this); !ret) + if (auto const ret = checkSign(rules, *this); !ret) return ret; if (isFieldPresent(sfCounterpartySignature)) { auto const counterSig = getFieldObject(sfCounterpartySignature); - if (auto const ret = checkSign(requireCanonicalSig, rules, counterSig); + if (auto const ret = checkSign(rules, counterSig); !ret) return Unexpected("Counterparty: " + ret.error()); } @@ -278,9 +272,7 @@ STTx::checkSign( } Expected -STTx::checkBatchSign( - RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules) const +STTx::checkBatchSign(Rules const& rules) const { try { @@ -297,8 +289,8 @@ STTx::checkBatchSign( { Blob const& signingPubKey = signer.getFieldVL(sfSigningPubKey); auto const result = signingPubKey.empty() - ? checkBatchMultiSign(signer, requireCanonicalSig, rules) - : checkBatchSingleSign(signer, requireCanonicalSig); + ? checkBatchMultiSign(signer, rules) + : checkBatchSingleSign(signer); if (!result) return result; @@ -393,10 +385,7 @@ STTx::getMetaSQL( } static Expected -singleSignHelper( - STObject const& sigObject, - Slice const& data, - bool const fullyCanonical) +singleSignHelper(STObject const& sigObject, Slice const& data) { // We don't allow both a non-empty sfSigningPubKey and an sfSigners. // That would allow the transaction to be signed two ways. So if both @@ -411,11 +400,8 @@ singleSignHelper( if (publicKeyType(makeSlice(spk))) { Blob const signature = sigObject.getFieldVL(sfTxnSignature); - validSig = verify( - PublicKey(makeSlice(spk)), - data, - makeSlice(signature), - fullyCanonical); + validSig = + verify(PublicKey(makeSlice(spk)), data, makeSlice(signature)); } } catch (std::exception const&) @@ -430,33 +416,24 @@ singleSignHelper( } Expected -STTx::checkSingleSign( - RequireFullyCanonicalSig requireCanonicalSig, - STObject const& sigObject) const +STTx::checkSingleSign(STObject const& sigObject) const { auto const data = getSigningData(*this); - bool const fullyCanonical = (getFlags() & tfFullyCanonicalSig) || - (requireCanonicalSig == STTx::RequireFullyCanonicalSig::yes); - return singleSignHelper(sigObject, makeSlice(data), fullyCanonical); + return singleSignHelper(sigObject, makeSlice(data)); } Expected -STTx::checkBatchSingleSign( - STObject const& batchSigner, - RequireFullyCanonicalSig requireCanonicalSig) const +STTx::checkBatchSingleSign(STObject const& batchSigner) const { Serializer msg; serializeBatch(msg, getFlags(), getBatchTransactionIDs()); - bool const fullyCanonical = (getFlags() & tfFullyCanonicalSig) || - (requireCanonicalSig == STTx::RequireFullyCanonicalSig::yes); - return singleSignHelper(batchSigner, msg.slice(), fullyCanonical); + return singleSignHelper(batchSigner, msg.slice()); } Expected multiSignHelper( STObject const& sigObject, std::optional txnAccountID, - bool const fullyCanonical, std::function makeMsg, Rules const& rules) { @@ -513,8 +490,7 @@ multiSignHelper( validSig = verify( PublicKey(makeSlice(spk)), makeMsg(accountID).slice(), - makeSlice(signature), - fullyCanonical); + makeSlice(signature)); } } catch (std::exception const& e) @@ -533,14 +509,8 @@ multiSignHelper( } Expected -STTx::checkBatchMultiSign( - STObject const& batchSigner, - RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules) const +STTx::checkBatchMultiSign(STObject const& batchSigner, Rules const& rules) const { - bool const fullyCanonical = (getFlags() & tfFullyCanonicalSig) || - (requireCanonicalSig == RequireFullyCanonicalSig::yes); - // We can ease the computational load inside the loop a bit by // pre-constructing part of the data that we hash. Fill a Serializer // with the stuff that stays constant from signature to signature. @@ -549,7 +519,6 @@ STTx::checkBatchMultiSign( return multiSignHelper( batchSigner, std::nullopt, - fullyCanonical, [&dataStart](AccountID const& accountID) -> Serializer { Serializer s = dataStart; finishMultiSigningData(accountID, s); @@ -559,14 +528,8 @@ STTx::checkBatchMultiSign( } Expected -STTx::checkMultiSign( - RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules, - STObject const& sigObject) const +STTx::checkMultiSign(Rules const& rules, STObject const& sigObject) const { - bool const fullyCanonical = (getFlags() & tfFullyCanonicalSig) || - (requireCanonicalSig == RequireFullyCanonicalSig::yes); - // Used inside the loop in multiSignHelper to enforce that // the account owner may not multisign for themselves. auto const txnAccountID = &sigObject != this @@ -580,7 +543,6 @@ STTx::checkMultiSign( return multiSignHelper( sigObject, txnAccountID, - fullyCanonical, [&dataStart](AccountID const& accountID) -> Serializer { Serializer s = dataStart; finishMultiSigningData(accountID, s); diff --git a/src/test/app/AccountDelete_test.cpp b/src/test/app/AccountDelete_test.cpp index 9d5d1cd877..f22f9c4165 100644 --- a/src/test/app/AccountDelete_test.cpp +++ b/src/test/app/AccountDelete_test.cpp @@ -418,60 +418,6 @@ public: env.close(); } - void - testAmendmentEnable() - { - // Start with the featureDeletableAccounts amendment disabled. - // Then enable the amendment and delete an account. - using namespace jtx; - - testcase("Amendment enable"); - - Env env{*this, testable_amendments() - featureDeletableAccounts}; - Account const alice("alice"); - Account const becky("becky"); - - env.fund(XRP(10000), alice, becky); - env.close(); - - // Close enough ledgers to be able to delete alice's account. - incLgrSeqForAccDel(env, alice); - - // Verify that alice's account root is present. - Keylet const aliceAcctKey{keylet::account(alice.id())}; - BEAST_EXPECT(env.closed()->exists(aliceAcctKey)); - - auto const alicePreDelBal{env.balance(alice)}; - auto const beckyPreDelBal{env.balance(becky)}; - - auto const acctDelFee{drops(env.current()->fees().increment)}; - env(acctdelete(alice, becky), fee(acctDelFee), ter(temDISABLED)); - env.close(); - - // Verify that alice's account root is still present and alice and - // becky both have their XRP. - BEAST_EXPECT(env.current()->exists(aliceAcctKey)); - BEAST_EXPECT(env.balance(alice) == alicePreDelBal); - BEAST_EXPECT(env.balance(becky) == beckyPreDelBal); - - // When the amendment is enabled the previous transaction is - // retried into the new open ledger and succeeds. - env.enableFeature(featureDeletableAccounts); - env.close(); - - // alice's account is still in the most recently closed ledger. - BEAST_EXPECT(env.closed()->exists(aliceAcctKey)); - - // Verify that alice's account root is gone from the current ledger - // and becky has alice's XRP. - BEAST_EXPECT(!env.current()->exists(aliceAcctKey)); - BEAST_EXPECT( - env.balance(becky) == alicePreDelBal + beckyPreDelBal - acctDelFee); - - env.close(); - BEAST_EXPECT(!env.closed()->exists(aliceAcctKey)); - } - void testTooManyOffers() { @@ -1148,7 +1094,6 @@ public: testBasics(); testDirectories(); testOwnedTypes(); - testAmendmentEnable(); testTooManyOffers(); testImplicitlyCreatedTrustline(); testBalanceTooSmallForFee(); diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 12ac309acb..5f57322be1 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -5720,7 +5720,6 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite // Close the ledger until the ledger sequence is large enough to delete // the account (no longer within ) - // This is enforced by the featureDeletableAccounts amendment auto incLgrSeqForAcctDel = [&](Env& env, Account const& acct) { int const delta = [&]() -> int { if (env.seq(acct) + 255 > openLedgerSeq(env)) @@ -5839,8 +5838,6 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite } env.close(); - // Increment ledger sequence to the number that is - // enforced by the featureDeletableAccounts amendment incLgrSeqForAcctDel(env, alice); // Verify that alice's account root is present. @@ -5943,8 +5940,6 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite BEAST_EXPECT(ticketCount(env, alice) == 0); - // Increment ledger sequence to the number that is - // enforced by the featureDeletableAccounts amendment incLgrSeqForAcctDel(env, alice); // Verify that alice's account root is present. @@ -6053,8 +6048,6 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite BEAST_EXPECT(ticketCount(env, minter) == 0); - // Increment ledger sequence to the number that is - // enforced by the featureDeletableAccounts amendment incLgrSeqForAcctDel(env, alice); // Verify that alice's account root is present. diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index 4b7f156738..8e78969397 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -4330,7 +4330,7 @@ public: Account const ellie("ellie"); Account const fiona("fiona"); - constexpr int ledgersInQueue = 10; + constexpr int ledgersInQueue = 20; auto cfg = makeConfig( {{"minimum_txn_in_ledger_standalone", "1"}, {"ledgers_in_queue", std::to_string(ledgersInQueue)}, diff --git a/src/test/app/tx/apply_test.cpp b/src/test/app/tx/apply_test.cpp index 236905cefd..308ad3d3b8 100644 --- a/src/test/app/tx/apply_test.cpp +++ b/src/test/app/tx/apply_test.cpp @@ -35,23 +35,6 @@ public: SerialIter sitTrans(makeSlice(*ret)); STTx const tx = *std::make_shared(std::ref(sitTrans)); - { - test::jtx::Env no_fully_canonical( - *this, - test::jtx::testable_amendments() - - featureRequireFullyCanonicalSig); - - Validity valid = checkValidity( - no_fully_canonical.app().getHashRouter(), - tx, - no_fully_canonical.current()->rules(), - no_fully_canonical.app().config()) - .first; - - if (valid != Validity::Valid) - fail("Non-Fully canonical signature was not permitted"); - } - { test::jtx::Env fully_canonical( *this, test::jtx::testable_amendments()); diff --git a/src/test/ledger/View_test.cpp b/src/test/ledger/View_test.cpp index b7dbf4ec9e..ab978fda59 100644 --- a/src/test/ledger/View_test.cpp +++ b/src/test/ledger/View_test.cpp @@ -1114,10 +1114,10 @@ class GetAmendments_test : public beast::unit_test::suite break; } - // There should be at least 5 amendments. Don't do exact comparison + // There should be at least 3 amendments. Don't do exact comparison // to avoid maintenance as more amendments are added in the future. BEAST_EXPECT(i == 254); - BEAST_EXPECT(majorities.size() >= 5); + BEAST_EXPECT(majorities.size() >= 3); // None of the amendments should be enabled yet. auto enableds = getEnabledAmendments(*env.closed()); @@ -1135,7 +1135,7 @@ class GetAmendments_test : public beast::unit_test::suite break; } BEAST_EXPECT(i == 255); - BEAST_EXPECT(enableds.size() >= 5); + BEAST_EXPECT(enableds.size() >= 3); } void diff --git a/src/test/protocol/STTx_test.cpp b/src/test/protocol/STTx_test.cpp index 8a434486ea..50cd8c511f 100644 --- a/src/test/protocol/STTx_test.cpp +++ b/src/test/protocol/STTx_test.cpp @@ -1615,8 +1615,7 @@ public: BEAST_EXPECT(!defaultRules.enabled(featureAMM)); unexpected( - !j.checkSign(STTx::RequireFullyCanonicalSig::yes, defaultRules), - "Transaction fails signature test"); + !j.checkSign(defaultRules), "Transaction fails signature test"); Serializer rawTxn; j.add(rawTxn); diff --git a/src/test/protocol/SecretKey_test.cpp b/src/test/protocol/SecretKey_test.cpp index bd7e2ccfe4..803988e9d5 100644 --- a/src/test/protocol/SecretKey_test.cpp +++ b/src/test/protocol/SecretKey_test.cpp @@ -145,7 +145,7 @@ public: auto sig = sign(pk, sk, makeSlice(data)); BEAST_EXPECT(sig.size() != 0); - BEAST_EXPECT(verify(pk, makeSlice(data), sig, true)); + BEAST_EXPECT(verify(pk, makeSlice(data), sig)); // Construct wrong data: auto badData = data; @@ -156,17 +156,17 @@ public: std::max_element(badData.begin(), badData.end())); // Wrong data: should fail - BEAST_EXPECT(!verify(pk, makeSlice(badData), sig, true)); + BEAST_EXPECT(!verify(pk, makeSlice(badData), sig)); // Slightly change the signature: if (auto ptr = sig.data()) ptr[j % sig.size()]++; // Wrong signature: should fail - BEAST_EXPECT(!verify(pk, makeSlice(data), sig, true)); + BEAST_EXPECT(!verify(pk, makeSlice(data), sig)); // Wrong data and signature: should fail - BEAST_EXPECT(!verify(pk, makeSlice(badData), sig, true)); + BEAST_EXPECT(!verify(pk, makeSlice(badData), sig)); } } } diff --git a/src/test/rpc/Feature_test.cpp b/src/test/rpc/Feature_test.cpp index 10f82ac88d..e01f7566ac 100644 --- a/src/test/rpc/Feature_test.cpp +++ b/src/test/rpc/Feature_test.cpp @@ -124,8 +124,7 @@ class Feature_test : public beast::unit_test::suite featureToName(fixRemoveNFTokenAutoTrustLine) == "fixRemoveNFTokenAutoTrustLine"); BEAST_EXPECT(featureToName(featureFlow) == "Flow"); - BEAST_EXPECT( - featureToName(featureDeletableAccounts) == "DeletableAccounts"); + BEAST_EXPECT(featureToName(featureDID) == "DID"); BEAST_EXPECT( featureToName(fixIncludeKeyletFields) == "fixIncludeKeyletFields"); BEAST_EXPECT(featureToName(featureTokenEscrow) == "TokenEscrow"); @@ -184,16 +183,16 @@ class Feature_test : public beast::unit_test::suite using namespace test::jtx; Env env{*this}; - auto jrr = env.rpc("feature", "RequireFullyCanonicalSig")[jss::result]; + auto jrr = env.rpc("feature", "Flow")[jss::result]; BEAST_EXPECTS(jrr[jss::status] == jss::success, "status"); jrr.removeMember(jss::status); BEAST_EXPECT(jrr.size() == 1); BEAST_EXPECT( - jrr.isMember("00C1FC4A53E60AB02C864641002B3172F38677E29C26C54066851" - "79B37E1EDAC")); + jrr.isMember("740352F2412A9909880C23A559FCECEDA3BE2126FED62FC7660D6" + "28A06927F11")); auto feature = *(jrr.begin()); - BEAST_EXPECTS(feature[jss::name] == "RequireFullyCanonicalSig", "name"); + BEAST_EXPECTS(feature[jss::name] == "Flow", "name"); BEAST_EXPECTS(!feature[jss::enabled].asBool(), "enabled"); BEAST_EXPECTS( feature[jss::vetoed].isBool() && !feature[jss::vetoed].asBool(), @@ -201,7 +200,7 @@ class Feature_test : public beast::unit_test::suite BEAST_EXPECTS(feature[jss::supported].asBool(), "supported"); // feature names are case-sensitive - expect error here - jrr = env.rpc("feature", "requireFullyCanonicalSig")[jss::result]; + jrr = env.rpc("feature", "flow")[jss::result]; BEAST_EXPECT(jrr[jss::error] == "badFeature"); BEAST_EXPECT(jrr[jss::error_message] == "Feature unknown or invalid."); } @@ -420,9 +419,9 @@ class Feature_test : public beast::unit_test::suite break; } - // There should be at least 5 amendments. Don't do exact comparison + // There should be at least 3 amendments. Don't do exact comparison // to avoid maintenance as more amendments are added in the future. - BEAST_EXPECT(majorities.size() >= 5); + BEAST_EXPECT(majorities.size() >= 3); std::map const& votes = ripple::detail::supportedAmendments(); @@ -477,8 +476,8 @@ class Feature_test : public beast::unit_test::suite testcase("Veto"); using namespace test::jtx; - Env env{*this, FeatureBitset{featureRequireFullyCanonicalSig}}; - constexpr char const* featureName = "RequireFullyCanonicalSig"; + Env env{*this, FeatureBitset{featureFlow}}; + constexpr char const* featureName = "Flow"; auto jrr = env.rpc("feature", featureName)[jss::result]; if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status")) @@ -529,7 +528,22 @@ class Feature_test : public beast::unit_test::suite using namespace test::jtx; Env env{*this}; - constexpr char const* featureName = "CryptoConditionsSuite"; + + auto const& supportedAmendments = detail::supportedAmendments(); + auto obsoleteFeature = std::find_if( + std::begin(supportedAmendments), + std::end(supportedAmendments), + [](auto const& pair) { + return pair.second == VoteBehavior::Obsolete; + }); + + if (obsoleteFeature == std::end(supportedAmendments)) + { + pass(); + return; + } + + auto const featureName = obsoleteFeature->first; auto jrr = env.rpc("feature", featureName)[jss::result]; if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status")) diff --git a/src/test/rpc/LedgerClosed_test.cpp b/src/test/rpc/LedgerClosed_test.cpp index b3a5e60afd..ba25a115c3 100644 --- a/src/test/rpc/LedgerClosed_test.cpp +++ b/src/test/rpc/LedgerClosed_test.cpp @@ -38,7 +38,7 @@ public: lc_result = env.rpc("ledger_closed")[jss::result]; BEAST_EXPECT( lc_result[jss::ledger_hash] == - "E86DE7F3D7A4D9CE17EF7C8BA08A8F4D8F643B9552F0D895A31CDA78F541DE4E"); + "0F1A9E0C109ADEF6DA2BDE19217C12BBEC57174CBDBD212B0EBDC1CEDB853185"); BEAST_EXPECT(lc_result[jss::ledger_index] == 3); } diff --git a/src/test/rpc/LedgerRPC_test.cpp b/src/test/rpc/LedgerRPC_test.cpp index 2d9daa5d9d..57a20527d0 100644 --- a/src/test/rpc/LedgerRPC_test.cpp +++ b/src/test/rpc/LedgerRPC_test.cpp @@ -307,8 +307,8 @@ class LedgerRPC_test : public beast::unit_test::suite { std::string const hash3{ - "E86DE7F3D7A4D9CE17EF7C8BA08A8F4D" - "8F643B9552F0D895A31CDA78F541DE4E"}; + "0F1A9E0C109ADEF6DA2BDE19217C12BBEC57174CBDBD212B0EBDC1CEDB8531" + "85"}; // access via the ledger_hash field Json::Value jvParams; jvParams[jss::ledger_hash] = hash3; diff --git a/src/test/rpc/LedgerRequestRPC_test.cpp b/src/test/rpc/LedgerRequestRPC_test.cpp index 5917dcedf2..0751b120aa 100644 --- a/src/test/rpc/LedgerRequestRPC_test.cpp +++ b/src/test/rpc/LedgerRequestRPC_test.cpp @@ -200,7 +200,7 @@ public: result = env.rpc("ledger_request", "3")[jss::result]; constexpr char const* hash3 = - "8D631B20BC989AF568FBA97375290544B0703A5ADC1CF9E9053580461690C9EE"; + "9FFD8AE09190D5002FE4252A1B29EABCF40DABBCE3B42619C6BD0BE381D51103"; BEAST_EXPECT(result[jss::ledger][jss::ledger_index] == "3"); BEAST_EXPECT( result[jss::ledger][jss::total_coins] == "99999999999999980"); @@ -209,14 +209,14 @@ public: BEAST_EXPECT(result[jss::ledger][jss::parent_hash] == hash2); BEAST_EXPECT( result[jss::ledger][jss::account_hash] == - "BC9EF2A16BFF80BCFABA6FA84688D858D33BD0FA0435CAA9DF6DA4105A39A29E"); + "35738B8517F37D08983AF6BC7DA483CCA9CF6B41B1FECB31A20028D78FE0BB22"); BEAST_EXPECT( result[jss::ledger][jss::transaction_hash] == - "0213EC486C058B3942FBE3DAC6839949A5C5B02B8B4244C8998EFDF04DBD8222"); + "CBD7F0948EBFA2241DE4EA627939A0FFEE6B80A90FE09C42C825DA546E9B73FF"); result = env.rpc("ledger_request", "4")[jss::result]; constexpr char const* hash4 = - "1A8E7098B23597E73094DADA58C9D62F3AB93A12C6F7666D56CA85A6CFDE530F"; + "7C9B614445517B8C6477E0AB10A35FFC1A23A34FEA41A91ECBDE884CC097C6E1"; BEAST_EXPECT(result[jss::ledger][jss::ledger_index] == "4"); BEAST_EXPECT( result[jss::ledger][jss::total_coins] == "99999999999999960"); @@ -225,14 +225,14 @@ public: BEAST_EXPECT(result[jss::ledger][jss::parent_hash] == hash3); BEAST_EXPECT( result[jss::ledger][jss::account_hash] == - "C690188F123C91355ADA8BDF4AC5B5C927076D3590C215096868A5255264C6DD"); + "1EE701DD2A150205173E1EDE8D474DF6803EC95253DAAEE965B9D896CFC32A04"); BEAST_EXPECT( result[jss::ledger][jss::transaction_hash] == - "3CBDB8F42E04333E1642166BFB93AC9A7E1C6C067092CD5D881D6F3AB3D67E76"); + "9BBDDBF926100DFFF364E16268F544B19F5B9BC6ECCBBC104F98D13FA9F3BC35"); result = env.rpc("ledger_request", "5")[jss::result]; constexpr char const* hash5 = - "C6A222D71AE65D7B4F240009EAD5DEB20D7EEDE5A4064F28BBDBFEEB6FBE48E5"; + "98885D02145CCE4AD2605F1809F17188DB2053B14ED399CAC985DD8E03DCA8C0"; BEAST_EXPECT(result[jss::ledger][jss::ledger_index] == "5"); BEAST_EXPECT( result[jss::ledger][jss::total_coins] == "99999999999999940"); @@ -241,10 +241,10 @@ public: BEAST_EXPECT(result[jss::ledger][jss::parent_hash] == hash4); BEAST_EXPECT( result[jss::ledger][jss::account_hash] == - "EA81CD9D36740736F00CB747E0D0E32D3C10B695823D961F0FB9A1CE7133DD4D"); + "41D64D64796468DEA7AE2A7282C0BB525D6FD7ABC29453C5E5BC6406E947CBCE"); BEAST_EXPECT( result[jss::ledger][jss::transaction_hash] == - "C3D086CD6BDB9E97AD1D513B2C049EF2840BD21D0B3E22D84EBBB89B6D2EF59D"); + "8FE8592EF22FBC2E8C774C7A1ED76AA3FCE64BED17D748CBA9AFDF7072FE36C7"); result = env.rpc("ledger_request", "6")[jss::result]; BEAST_EXPECT(result[jss::error] == "invalidParams"); diff --git a/src/xrpld/app/rdb/backend/detail/Node.cpp b/src/xrpld/app/rdb/backend/detail/Node.cpp index ed8c5c06aa..04f328390d 100644 --- a/src/xrpld/app/rdb/backend/detail/Node.cpp +++ b/src/xrpld/app/rdb/backend/detail/Node.cpp @@ -171,7 +171,7 @@ getRowsMinMax(soci::session& session, TableType type) bool saveValidatedLedger( DatabaseCon& ldgDB, - DatabaseCon& txnDB, + std::unique_ptr const& txnDB, Application& app, std::shared_ptr const& ledger, bool current) @@ -254,7 +254,15 @@ saveValidatedLedger( if (app.config().useTxTables()) { - auto db = txnDB.checkoutDb(); + if (!txnDB) + { + // LCOV_EXCL_START + JLOG(j.fatal()) << "TxTables db isn't available"; + Throw("TxTables db isn't available"); + // LCOV_EXCL_STOP + } + + auto db = txnDB->checkoutDb(); soci::transaction tr(*db); diff --git a/src/xrpld/app/rdb/backend/detail/Node.h b/src/xrpld/app/rdb/backend/detail/Node.h index 412631571e..2c0fd69445 100644 --- a/src/xrpld/app/rdb/backend/detail/Node.h +++ b/src/xrpld/app/rdb/backend/detail/Node.h @@ -111,7 +111,7 @@ getRowsMinMax(soci::session& session, TableType type); bool saveValidatedLedger( DatabaseCon& ldgDB, - DatabaseCon& txnDB, + std::unique_ptr const& txnDB, Application& app, std::shared_ptr const& ledger, bool current); diff --git a/src/xrpld/app/rdb/backend/detail/SQLiteDatabase.cpp b/src/xrpld/app/rdb/backend/detail/SQLiteDatabase.cpp index 4934205bfd..944ed814f5 100644 --- a/src/xrpld/app/rdb/backend/detail/SQLiteDatabase.cpp +++ b/src/xrpld/app/rdb/backend/detail/SQLiteDatabase.cpp @@ -392,8 +392,7 @@ SQLiteDatabaseImp::saveValidatedLedger( { if (existsLedger()) { - if (!detail::saveValidatedLedger( - *lgrdb_, *txdb_, app_, ledger, current)) + if (!detail::saveValidatedLedger(*lgrdb_, txdb_, app_, ledger, current)) return false; } diff --git a/src/xrpld/app/tx/detail/Batch.cpp b/src/xrpld/app/tx/detail/Batch.cpp index 9d2fe4e47d..8bf0c7fe02 100644 --- a/src/xrpld/app/tx/detail/Batch.cpp +++ b/src/xrpld/app/tx/detail/Batch.cpp @@ -456,8 +456,7 @@ Batch::preflightSigValidated(PreflightContext const& ctx) } // Check the batch signers signatures. - auto const sigResult = ctx.tx.checkBatchSign( - STTx::RequireFullyCanonicalSig::yes, ctx.rules); + auto const sigResult = ctx.tx.checkBatchSign(ctx.rules); if (!sigResult) { diff --git a/src/xrpld/app/tx/detail/DeleteAccount.cpp b/src/xrpld/app/tx/detail/DeleteAccount.cpp index 0654c8dbce..c9fd0cc75e 100644 --- a/src/xrpld/app/tx/detail/DeleteAccount.cpp +++ b/src/xrpld/app/tx/detail/DeleteAccount.cpp @@ -22,9 +22,6 @@ namespace ripple { bool DeleteAccount::checkExtraFeatures(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureDeletableAccounts)) - return false; - if (ctx.tx.isFieldPresent(sfCredentialIDs) && !ctx.rules.enabled(featureCredentials)) return false; diff --git a/src/xrpld/app/tx/detail/Escrow.cpp b/src/xrpld/app/tx/detail/Escrow.cpp index 5cf90809a2..c22b8145c6 100644 --- a/src/xrpld/app/tx/detail/Escrow.cpp +++ b/src/xrpld/app/tx/detail/Escrow.cpp @@ -153,12 +153,6 @@ EscrowCreate::preflight(PreflightContext const& ctx) << ec.message(); return temMALFORMED; } - - // Conditions other than PrefixSha256 require the - // "CryptoConditionsSuite" amendment: - if (condition->type != Type::preimageSha256 && - !ctx.rules.enabled(featureCryptoConditionsSuite)) - return temDISABLED; } return tesSUCCESS; diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index c5bdbcf6dc..04f28352b8 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -1057,12 +1057,7 @@ ValidNewAccountRoot::finalize( return false; } - std::uint32_t const startingSeq = // - pseudoAccount // - ? 0 // - : view.rules().enabled(featureDeletableAccounts) // - ? view.seq() // - : 1; + std::uint32_t const startingSeq = pseudoAccount ? 0 : view.seq(); if (accountSeq_ != startingSeq) { diff --git a/src/xrpld/app/tx/detail/PayChan.cpp b/src/xrpld/app/tx/detail/PayChan.cpp index b52ae78523..ae47036ab6 100644 --- a/src/xrpld/app/tx/detail/PayChan.cpp +++ b/src/xrpld/app/tx/detail/PayChan.cpp @@ -440,7 +440,7 @@ PayChanClaim::preflight(PreflightContext const& ctx) PublicKey const pk(ctx.tx[sfPublicKey]); Serializer msg; serializePayChanAuthorization(msg, k.key, authAmt); - if (!verify(pk, msg.slice(), *sig, /*canonical*/ true)) + if (!verify(pk, msg.slice(), *sig)) return temBAD_SIGNATURE; } diff --git a/src/xrpld/app/tx/detail/Payment.cpp b/src/xrpld/app/tx/detail/Payment.cpp index 3aff4db5e1..56aeb1b0aa 100644 --- a/src/xrpld/app/tx/detail/Payment.cpp +++ b/src/xrpld/app/tx/detail/Payment.cpp @@ -398,14 +398,10 @@ Payment::doApply() if (!sleDst) { - std::uint32_t const seqno{ - view().rules().enabled(featureDeletableAccounts) ? view().seq() - : 1}; - // Create the account. sleDst = std::make_shared(k); sleDst->setAccountID(sfAccount, dstAccountID); - sleDst->setFieldU32(sfSequence, seqno); + sleDst->setFieldU32(sfSequence, view().seq()); view().insert(sleDst); } diff --git a/src/xrpld/app/tx/detail/VaultWithdraw.cpp b/src/xrpld/app/tx/detail/VaultWithdraw.cpp index 99a5f5f721..a3bef88d49 100644 --- a/src/xrpld/app/tx/detail/VaultWithdraw.cpp +++ b/src/xrpld/app/tx/detail/VaultWithdraw.cpp @@ -83,6 +83,8 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx) if (auto const ret = checkFrozen(ctx.view, dstAcct, vaultAsset)) return ret; + // Cannot return shares to the vault, if the underlying asset was frozen for + // the submitter if (auto const ret = checkFrozen(ctx.view, account, vaultShare)) return ret; diff --git a/src/xrpld/app/tx/detail/XChainBridge.cpp b/src/xrpld/app/tx/detail/XChainBridge.cpp index 554d4a8436..e555dca0a1 100644 --- a/src/xrpld/app/tx/detail/XChainBridge.cpp +++ b/src/xrpld/app/tx/detail/XChainBridge.cpp @@ -464,12 +464,9 @@ transferHelper( } // Create the account. - std::uint32_t const seqno{ - psb.rules().enabled(featureDeletableAccounts) ? psb.seq() : 1}; - sleDst = std::make_shared(dstK); sleDst->setAccountID(sfAccount, dst); - sleDst->setFieldU32(sfSequence, seqno); + sleDst->setFieldU32(sfSequence, psb.seq()); psb.insert(sleDst); } diff --git a/src/xrpld/app/tx/detail/apply.cpp b/src/xrpld/app/tx/detail/apply.cpp index e9709c20f5..06be378c2b 100644 --- a/src/xrpld/app/tx/detail/apply.cpp +++ b/src/xrpld/app/tx/detail/apply.cpp @@ -58,13 +58,7 @@ checkValidity( if (!any(flags & SF_SIGGOOD)) { - // Don't know signature state. Check it. - auto const requireCanonicalSig = - rules.enabled(featureRequireFullyCanonicalSig) - ? STTx::RequireFullyCanonicalSig::yes - : STTx::RequireFullyCanonicalSig::no; - - auto const sigVerify = tx.checkSign(requireCanonicalSig, rules); + auto const sigVerify = tx.checkSign(rules); if (!sigVerify) { router.setFlags(id, SF_SIGBAD); diff --git a/src/xrpld/rpc/handlers/PayChanClaim.cpp b/src/xrpld/rpc/handlers/PayChanClaim.cpp index dd36a873f0..95c4ba5f3d 100644 --- a/src/xrpld/rpc/handlers/PayChanClaim.cpp +++ b/src/xrpld/rpc/handlers/PayChanClaim.cpp @@ -137,8 +137,7 @@ doChannelVerify(RPC::JsonContext& context) serializePayChanAuthorization(msg, channelId, XRPAmount(drops)); Json::Value result; - result[jss::signature_verified] = - verify(*pk, msg.slice(), makeSlice(*sig), /*canonical*/ true); + result[jss::signature_verified] = verify(*pk, msg.slice(), makeSlice(*sig)); return result; }