From 6e4945c56b1a1c063b32921d7750607587ec3063 Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Wed, 15 Jan 2020 14:41:21 -0800 Subject: [PATCH] Remove conditionals for featureMultiSign enabled 27Jun2016 --- src/ripple/app/tx/impl/SetAccount.cpp | 7 +-- src/ripple/app/tx/impl/SetSignerList.cpp | 3 -- src/ripple/app/tx/impl/Transactor.cpp | 10 ++-- src/ripple/app/tx/impl/apply.cpp | 5 +- src/ripple/protocol/Feature.h | 3 +- src/ripple/protocol/STTx.h | 2 +- src/ripple/protocol/impl/Feature.cpp | 3 +- src/ripple/protocol/impl/STTx.cpp | 20 +++----- src/ripple/rpc/handlers/SignFor.cpp | 7 --- src/ripple/rpc/handlers/SubmitMultiSigned.cpp | 7 --- src/test/app/Check_test.cpp | 4 +- src/test/app/MultiSign_test.cpp | 51 ------------------- src/test/protocol/STTx_test.cpp | 2 +- src/test/rpc/AccountSet_test.cpp | 13 ++--- 14 files changed, 21 insertions(+), 116 deletions(-) diff --git a/src/ripple/app/tx/impl/SetAccount.cpp b/src/ripple/app/tx/impl/SetAccount.cpp index 0bf8060604b..93c27a616f4 100644 --- a/src/ripple/app/tx/impl/SetAccount.cpp +++ b/src/ripple/app/tx/impl/SetAccount.cpp @@ -295,12 +295,7 @@ SetAccount::doApply () (!view().peek (keylet::signers (account_)))) { // Account has no regular key or multi-signer signer list. - - // Prevent transaction changes until we're ready. - if (view().rules().enabled(featureMultiSign)) - return tecNO_ALTERNATIVE_KEY; - - return tecNO_REGULAR_KEY; + return tecNO_ALTERNATIVE_KEY; } JLOG(j_.trace()) << "Set lsfDisableMaster."; diff --git a/src/ripple/app/tx/impl/SetSignerList.cpp b/src/ripple/app/tx/impl/SetSignerList.cpp index 190fd0c4f1a..76501ea6ed8 100644 --- a/src/ripple/app/tx/impl/SetSignerList.cpp +++ b/src/ripple/app/tx/impl/SetSignerList.cpp @@ -74,9 +74,6 @@ SetSignerList::determineOperation(STTx const& tx, NotTEC SetSignerList::preflight (PreflightContext const& ctx) { - if (! ctx.rules.enabled(featureMultiSign)) - return temDISABLED; - auto const ret = preflight1 (ctx); if (!isTesSuccess (ret)) return ret; diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index bd52c826075..9a301514461 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -331,13 +331,9 @@ TER Transactor::apply () NotTEC Transactor::checkSign (PreclaimContext const& ctx) { - // Make sure multisigning is enabled before we check for multisignatures. - if (ctx.view.rules().enabled(featureMultiSign)) - { - // If the pk is empty, then we must be multi-signing. - if (ctx.tx.getSigningPubKey().empty ()) - return checkMultiSign (ctx); - } + // If the pk is empty, then we must be multi-signing. + if (ctx.tx.getSigningPubKey().empty ()) + return checkMultiSign (ctx); return checkSingleSign (ctx); } diff --git a/src/ripple/app/tx/impl/apply.cpp b/src/ripple/app/tx/impl/apply.cpp index 7be537bf2b4..5f0d6c6a88d 100644 --- a/src/ripple/app/tx/impl/apply.cpp +++ b/src/ripple/app/tx/impl/apply.cpp @@ -38,9 +38,6 @@ checkValidity(HashRouter& router, STTx const& tx, Rules const& rules, Config const& config) { - auto const allowMultiSign = - rules.enabled(featureMultiSign); - auto const id = tx.getTransactionID(); auto const flags = router.getFlags(id); if (flags & SF_SIGBAD) @@ -50,7 +47,7 @@ checkValidity(HashRouter& router, if (!(flags & SF_SIGGOOD)) { // Don't know signature state. Check it. - auto const sigVerify = tx.checkSign(allowMultiSign); + auto const sigVerify = tx.checkSign(); if (! sigVerify.first) { router.setFlags(id, SF_SIGBAD); diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 60bee93ff67..66b95e3530d 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -50,7 +50,7 @@ class FeatureCollections { static constexpr char const* const featureNames[] = { - "MultiSign", + "MultiSign", // Unconditionally supported. "Tickets", "TrustSetAuth", "FeeEscalation", // Unconditionally supported. @@ -339,7 +339,6 @@ foreachFeature(FeatureBitset bs, F&& f) f(bitsetIndexToFeature(i)); } -extern uint256 const featureMultiSign; extern uint256 const featureTickets; extern uint256 const featureTrustSetAuth; extern uint256 const featureOwnerPaysFee; diff --git a/src/ripple/protocol/STTx.h b/src/ripple/protocol/STTx.h index de6ff5f97cb..8c4e7ae0923 100644 --- a/src/ripple/protocol/STTx.h +++ b/src/ripple/protocol/STTx.h @@ -133,7 +133,7 @@ class STTx final @return `true` if valid signature. If invalid, the error message string. */ std::pair - checkSign(bool allowMultiSign) const; + checkSign() const; // SQL Functions with metadata. static diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 22645e4b15e..6f0c51fdbf4 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -92,7 +92,7 @@ detail::supportedAmendments () // Removing them will cause servers to become amendment blocked. static std::vector const supported { - "MultiSign", + "MultiSign", // Unconditionally supported. // "Tickets", "TrustSetAuth", "FeeEscalation", // Unconditionally supported. @@ -151,7 +151,6 @@ uint256 bitsetIndexToFeature(size_t i) } -uint256 const featureMultiSign = *getRegisteredFeature("MultiSign"); uint256 const featureTickets = *getRegisteredFeature("Tickets"); uint256 const featureTrustSetAuth = *getRegisteredFeature("TrustSetAuth"); uint256 const featureOwnerPaysFee = *getRegisteredFeature("OwnerPaysFee"); diff --git a/src/ripple/protocol/impl/STTx.cpp b/src/ripple/protocol/impl/STTx.cpp index c9f835db296..ffdfed55554 100644 --- a/src/ripple/protocol/impl/STTx.cpp +++ b/src/ripple/protocol/impl/STTx.cpp @@ -177,24 +177,16 @@ void STTx::sign ( tid_ = getHash(HashPrefix::transactionID); } -std::pair STTx::checkSign(bool allowMultiSign) const +std::pair STTx::checkSign() const { std::pair ret {false, ""}; try { - if (allowMultiSign) - { - // Determine whether we're single- or multi-signing by looking - // at the SigningPubKey. It it's empty we must be - // multi-signing. Otherwise we're single-signing. - Blob const& signingPubKey = getFieldVL (sfSigningPubKey); - ret = signingPubKey.empty () ? - checkMultiSign () : checkSingleSign (); - } - else - { - ret = checkSingleSign (); - } + // Determine whether we're single- or multi-signing by looking + // at the SigningPubKey. It it's empty we must be + // multi-signing. Otherwise we're single-signing. + Blob const& signingPubKey = getFieldVL (sfSigningPubKey); + ret = signingPubKey.empty () ? checkMultiSign () : checkSingleSign (); } catch (std::exception const&) { diff --git a/src/ripple/rpc/handlers/SignFor.cpp b/src/ripple/rpc/handlers/SignFor.cpp index 29b99f85963..b63e847816e 100644 --- a/src/ripple/rpc/handlers/SignFor.cpp +++ b/src/ripple/rpc/handlers/SignFor.cpp @@ -39,13 +39,6 @@ Json::Value doSignFor (RPC::JsonContext& context) "Signing is not supported by this server."); } - // Bail if multisign is not enabled. - if (! context.app.getLedgerMaster().getValidatedRules(). - enabled (featureMultiSign)) - { - RPC::inject_error (rpcNOT_ENABLED, context.params); - return context.params; - } context.loadType = Resource::feeHighBurdenRPC; auto const failHard = context.params[jss::fail_hard].asBool(); auto const failType = NetworkOPs::doFailHard (failHard); diff --git a/src/ripple/rpc/handlers/SubmitMultiSigned.cpp b/src/ripple/rpc/handlers/SubmitMultiSigned.cpp index 274a106b6f1..c8722f14078 100644 --- a/src/ripple/rpc/handlers/SubmitMultiSigned.cpp +++ b/src/ripple/rpc/handlers/SubmitMultiSigned.cpp @@ -32,13 +32,6 @@ namespace ripple { // } Json::Value doSubmitMultiSigned (RPC::JsonContext& context) { - // Bail if multisign is not enabled. - if (! context.app.getLedgerMaster().getValidatedRules(). - enabled (featureMultiSign)) - { - RPC::inject_error (rpcNOT_ENABLED, context.params); - return context.params; - } context.loadType = Resource::feeHighBurdenRPC; auto const failHard = context.params[jss::fail_hard].asBool(); auto const failType = NetworkOPs::doFailHard (failHard); diff --git a/src/test/app/Check_test.cpp b/src/test/app/Check_test.cpp index 773532adf89..1480a2f0e81 100644 --- a/src/test/app/Check_test.cpp +++ b/src/test/app/Check_test.cpp @@ -892,7 +892,7 @@ class Check_test : public beast::unit_test::suite } // Use a regular key and also multisign to cash a check. - // featureMultiSign changes the reserve on a SignerList, so + // featureMultiSignReserve changes the reserve on a SignerList, so // check both before and after. FeatureBitset const allSupported {supported_amendments()}; for (auto const features : @@ -1541,7 +1541,7 @@ class Check_test : public beast::unit_test::suite Account const zoe {"zoe"}; IOU const USD {gw["USD"]}; - // featureMultiSign changes the reserve on a SignerList, so + // featureMultiSignReserve changes the reserve on a SignerList, so // check both before and after. FeatureBitset const allSupported {supported_amendments()}; for (auto const features : diff --git a/src/test/app/MultiSign_test.cpp b/src/test/app/MultiSign_test.cpp index a20c112637c..972fc1bdcc2 100644 --- a/src/test/app/MultiSign_test.cpp +++ b/src/test/app/MultiSign_test.cpp @@ -204,56 +204,6 @@ class MultiSign_test : public beast::unit_test::suite BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); } - void - test_enablement (FeatureBitset features) - { - testcase ("Enablement"); - - using namespace jtx; - Env env(*this, envconfig([](std::unique_ptr cfg) - { - cfg->loadFromString ("[" SECTION_SIGNING_SUPPORT "]\ntrue"); - return cfg; - }), features - featureMultiSign); - - Account const alice {"alice", KeyType::ed25519}; - env.fund(XRP(1000), alice); - env.close(); - - // NOTE: These six tests will fail if multisign is enabled. - env(signers(alice, 1, {{bogie, 1}}), ter(temDISABLED)); - env.close(); - env.require (owners (alice, 0)); - - std::uint32_t aliceSeq = env.seq (alice); - auto const baseFee = env.current()->fees().base; - env(noop(alice), msig(bogie), fee(2 * baseFee), ter(temINVALID)); - env.close(); - BEAST_EXPECT(env.seq(alice) == aliceSeq); - - env(signers(alice, 1, {{bogie, 1}, {demon,1}}), ter(temDISABLED)); - env.close(); - BEAST_EXPECT(env.seq(alice) == aliceSeq); - - { - Json::Value jvParams; - jvParams[jss::account] = alice.human(); - auto const jsmr = env.rpc("json", "submit_multisigned", to_string(jvParams))[jss::result]; - BEAST_EXPECT(jsmr[jss::error] == "notEnabled"); - BEAST_EXPECT(jsmr[jss::status] == "error"); - BEAST_EXPECT(jsmr[jss::error_message] == "Not enabled in configuration."); - } - - { - Json::Value jvParams; - jvParams[jss::account] = alice.human(); - auto const jsmr = env.rpc("json", "sign_for", to_string(jvParams))[jss::result]; - BEAST_EXPECT(jsmr[jss::error] == "notEnabled"); - BEAST_EXPECT(jsmr[jss::status] == "error"); - BEAST_EXPECT(jsmr[jss::error_message] == "Not enabled in configuration."); - } - } - void test_fee (FeatureBitset features) { testcase ("Fee"); @@ -1340,7 +1290,6 @@ class MultiSign_test : public beast::unit_test::suite test_noReserve (features); test_signerListSet (features); test_phantomSigners (features); - test_enablement (features); test_fee (features); test_misorderedSigners (features); test_masterSigners (features); diff --git a/src/test/protocol/STTx_test.cpp b/src/test/protocol/STTx_test.cpp index 0aecd8a7ebe..51a2de51ec1 100644 --- a/src/test/protocol/STTx_test.cpp +++ b/src/test/protocol/STTx_test.cpp @@ -1478,7 +1478,7 @@ class STTx_test : public beast::unit_test::suite }); j.sign (keypair.first, keypair.second); - unexpected (!j.checkSign (true).first, "Transaction fails signature test"); + unexpected (!j.checkSign().first, "Transaction fails signature test"); Serializer rawTxn; j.add (rawTxn); diff --git a/src/test/rpc/AccountSet_test.cpp b/src/test/rpc/AccountSet_test.cpp index dc933151d24..ca763263437 100644 --- a/src/test/rpc/AccountSet_test.cpp +++ b/src/test/rpc/AccountSet_test.cpp @@ -376,12 +376,10 @@ class AccountSet_test : public beast::unit_test::suite } } - void testBadInputs(bool withFeatures) + void testBadInputs() { using namespace test::jtx; - std::unique_ptr penv { - withFeatures ? new Env(*this) : new Env(*this, FeatureBitset{})}; - Env& env = *penv; + Env env (*this); Account const alice ("alice"); env.fund(XRP(10000), alice); @@ -415,7 +413,7 @@ class AccountSet_test : public beast::unit_test::suite env(fset (alice, asfDisableMaster), sig(alice), - ter(withFeatures ? tecNO_ALTERNATIVE_KEY : tecNO_REGULAR_KEY)); + ter(tecNO_ALTERNATIVE_KEY)); } void testRequireAuthWithDir() @@ -450,13 +448,10 @@ class AccountSet_test : public beast::unit_test::suite testMessageKey(); testWalletID(); testEmailHash(); - testBadInputs(true); - testBadInputs(false); + testBadInputs(); testRequireAuthWithDir(); testTransferRate(); } - - }; BEAST_DEFINE_TESTSUITE(AccountSet,app,ripple);