From a5583de6e6b2228e257e0d649702bb6a60ae5162 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Thu, 17 Dec 2015 15:35:56 -0500 Subject: [PATCH] Use features instead of ApplyFlags: tapENABLE_TESTING is removed from checks, and feature enablement is the sole method for activating features. Unit tests are updated to enable required features in the construction of the Env. Tickets are put on a feature switch instead of a build macro. --- src/BeastConfig.h | 7 ----- src/ripple/app/tests/MultiSign.test.cpp | 40 ++++++++++-------------- src/ripple/app/tests/Regression_test.cpp | 6 ++-- src/ripple/app/tests/SetAuth_test.cpp | 4 +-- src/ripple/app/tests/SusPay_test.cpp | 32 +++++++++++-------- src/ripple/app/tx/impl/CancelTicket.cpp | 6 ++-- src/ripple/app/tx/impl/CreateTicket.cpp | 6 ++-- src/ripple/app/tx/impl/SetAccount.cpp | 5 ++- src/ripple/app/tx/impl/SetSignerList.cpp | 5 ++- src/ripple/app/tx/impl/SetTrust.cpp | 4 +-- src/ripple/app/tx/impl/SusPay.cpp | 9 ++---- src/ripple/app/tx/impl/Transactor.cpp | 3 +- src/ripple/app/tx/impl/apply.cpp | 5 ++- src/ripple/protocol/Feature.h | 1 + src/ripple/protocol/impl/Feature.cpp | 2 +- src/ripple/rpc/tests/JSONRPC.test.cpp | 3 +- src/ripple/test/jtx/impl/Env_test.cpp | 7 ++--- 17 files changed, 64 insertions(+), 81 deletions(-) diff --git a/src/BeastConfig.h b/src/BeastConfig.h index 9a3ef01012..1ae8e44f2e 100644 --- a/src/BeastConfig.h +++ b/src/BeastConfig.h @@ -164,13 +164,6 @@ #define RIPPLE_SINGLE_IO_SERVICE_THREAD 0 #endif -/** Config: RIPPLE_ENABLE_TICKETS - Enables processing of ticket transactions -*/ -#ifndef RIPPLE_ENABLE_TICKETS -#define RIPPLE_ENABLE_TICKETS 0 -#endif - // Uses OpenSSL instead of alternatives #ifndef RIPPLE_USE_OPENSSL #define RIPPLE_USE_OPENSSL 1 diff --git a/src/ripple/app/tests/MultiSign.test.cpp b/src/ripple/app/tests/MultiSign.test.cpp index 205f2684da..a7a18f4bf3 100644 --- a/src/ripple/app/tests/MultiSign.test.cpp +++ b/src/ripple/app/tests/MultiSign.test.cpp @@ -17,6 +17,7 @@ #include #include // jss:: definitions +#include #include namespace ripple { @@ -38,7 +39,7 @@ public: void test_noReserve() { using namespace jtx; - Env env(*this); + Env env(*this, features(featureMultiSign)); Account const alice {"alice", KeyType::secp256k1}; // Pay alice enough to meet the initial reserve, but not enough to @@ -86,7 +87,7 @@ public: void test_signerListSet() { using namespace jtx; - Env env(*this); + Env env(*this, features(featureMultiSign)); Account const alice {"alice", KeyType::ed25519}; env.fund(XRP(1000), alice); @@ -131,7 +132,7 @@ public: void test_phantomSigners() { using namespace jtx; - Env env(*this); + Env env(*this, features(featureMultiSign)); Account const alice {"alice", KeyType::ed25519}; env.fund(XRP(1000), alice); env.close(); @@ -225,22 +226,15 @@ public: env.fund(XRP(1000), alice); env.close(); - // Add a signer list to alice. Should succeed. - env(signers(alice, 1, {{bogie, 1}})); + // Make sure multisign is disabled in production. + // NOTE: These six tests will fail when multisign is default enabled. + env.disable_testing(); + env(signers(alice, 1, {{bogie, 1}}), ter(temDISABLED)); env.close(); - env.require (owners (alice, 3)); + env.require (owners (alice, 0)); - // alice multisigns a transaction. Should succeed. std::uint32_t aliceSeq = env.seq (alice); auto const baseFee = env.app().config().FEE_DEFAULT; - env(noop(alice), msig(bogie), fee(2 * baseFee)); - env.close(); - expect (env.seq(alice) == aliceSeq + 1); - - // Make sure multisign is disabled in production. - // NOTE: These four tests will fail when multisign is default enabled. - env.disable_testing(); - aliceSeq = env.seq (alice); env(noop(alice), msig(bogie), fee(2 * baseFee), ter(temINVALID)); env.close(); expect (env.seq(alice) == aliceSeq); @@ -253,7 +247,7 @@ public: void test_fee () { using namespace jtx; - Env env(*this); + Env env(*this, features(featureMultiSign)); Account const alice {"alice", KeyType::ed25519}; env.fund(XRP(1000), alice); env.close(); @@ -303,7 +297,7 @@ public: void test_misorderedSigners() { using namespace jtx; - Env env(*this); + Env env(*this, features(featureMultiSign)); Account const alice {"alice", KeyType::ed25519}; env.fund(XRP(1000), alice); env.close(); @@ -325,7 +319,7 @@ public: void test_masterSigners() { using namespace jtx; - Env env(*this); + Env env(*this, features(featureMultiSign)); Account const alice {"alice", KeyType::ed25519}; Account const becky {"becky", KeyType::secp256k1}; Account const cheri {"cheri", KeyType::ed25519}; @@ -377,7 +371,7 @@ public: void test_regularSigners() { using namespace jtx; - Env env(*this); + Env env(*this, features(featureMultiSign)); Account const alice {"alice", KeyType::secp256k1}; Account const becky {"becky", KeyType::ed25519}; Account const cheri {"cheri", KeyType::secp256k1}; @@ -436,7 +430,7 @@ public: void test_heterogeneousSigners() { using namespace jtx; - Env env(*this); + Env env(*this, features(featureMultiSign)); Account const alice {"alice", KeyType::secp256k1}; Account const becky {"becky", KeyType::ed25519}; Account const cheri {"cheri", KeyType::secp256k1}; @@ -551,7 +545,7 @@ public: void test_keyDisable() { using namespace jtx; - Env env(*this); + Env env(*this, features(featureMultiSign)); Account const alice {"alice", KeyType::ed25519}; env.fund(XRP(1000), alice); @@ -626,7 +620,7 @@ public: void test_regKey() { using namespace jtx; - Env env(*this); + Env env(*this, features(featureMultiSign)); Account const alice {"alice", KeyType::secp256k1}; env.fund(XRP(1000), alice); @@ -658,7 +652,7 @@ public: void test_txTypes() { using namespace jtx; - Env env(*this); + Env env(*this, features(featureMultiSign)); Account const alice {"alice", KeyType::secp256k1}; Account const becky {"becky", KeyType::ed25519}; Account const zelda {"zelda", KeyType::secp256k1}; diff --git a/src/ripple/app/tests/Regression_test.cpp b/src/ripple/app/tests/Regression_test.cpp index e81bd32ba1..32a32832e8 100644 --- a/src/ripple/app/tests/Regression_test.cpp +++ b/src/ripple/app/tests/Regression_test.cpp @@ -66,8 +66,7 @@ struct Regression_test : public beast::unit_test::suite OpenView accum(&*next); auto const result = ripple::apply(env.app(), - accum, *jt.stx, tapENABLE_TESTING, - env.journal); + accum, *jt.stx, tapNONE, env.journal); expect(result.first == tesSUCCESS); expect(result.second); @@ -93,8 +92,7 @@ struct Regression_test : public beast::unit_test::suite OpenView accum(&*next); auto const result = ripple::apply(env.app(), - accum, *jt.stx, tapENABLE_TESTING, - env.journal); + accum, *jt.stx, tapNONE, env.journal); expect(result.first == tecINSUFF_FEE); expect(result.second); diff --git a/src/ripple/app/tests/SetAuth_test.cpp b/src/ripple/app/tests/SetAuth_test.cpp index 4352e69066..b7f5a81215 100644 --- a/src/ripple/app/tests/SetAuth_test.cpp +++ b/src/ripple/app/tests/SetAuth_test.cpp @@ -19,6 +19,7 @@ #include #include +#include #include namespace ripple { @@ -52,13 +53,12 @@ struct SetAuth_test : public beast::unit_test::suite auto const USD = gw["USD"]; { Env env(*this); - env.disable_testing(); env.fund(XRP(100000), "alice", gw); env(fset(gw, asfRequireAuth)); env(auth(gw, "alice", "USD"), ter(tecNO_LINE_REDUNDANT)); } { - Env env(*this); + Env env(*this, features(featureTrustSetAuth)); env.fund(XRP(100000), "alice", "bob", gw); env(fset(gw, asfRequireAuth)); env(auth(gw, "alice", "USD")); diff --git a/src/ripple/app/tests/SusPay_test.cpp b/src/ripple/app/tests/SusPay_test.cpp index 9d7dd90d79..7cc982dfd6 100644 --- a/src/ripple/app/tests/SusPay_test.cpp +++ b/src/ripple/app/tests/SusPay_test.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -145,15 +146,20 @@ struct SusPay_test : public beast::unit_test::suite using namespace jtx; using namespace std::chrono; using S = seconds; + auto const c = cond("receipt"); + { + Env env(*this, features(featureSusPay)); + auto T = [&env](NetClock::duration const& d) + { return env.now() + d; }; + env.fund(XRP(5000), "alice", "bob"); + // syntax + env(condpay("alice", "bob", XRP(1000), c.first, T(S{1}))); + } { Env env(*this); auto T = [&env](NetClock::duration const& d) { return env.now() + d; }; env.fund(XRP(5000), "alice", "bob"); - auto const c = cond("receipt"); - // syntax - env(condpay("alice", "bob", XRP(1000), c.first, T(S{1}))); - env.disable_testing(); // disabled in production env(condpay("alice", "bob", XRP(1000), c.first, T(S{1})), ter(temDISABLED)); env(finish("bob", "alice", 1), ter(temDISABLED)); @@ -168,7 +174,7 @@ struct SusPay_test : public beast::unit_test::suite using namespace std::chrono; using S = seconds; { - Env env(*this); + Env env(*this, features(featureSusPay)); auto const alice = Account("alice"); auto T = [&env](NetClock::duration const& d) { return env.now() + d; }; @@ -190,7 +196,7 @@ struct SusPay_test : public beast::unit_test::suite using namespace std::chrono; using S = seconds; { - Env env(*this); + Env env(*this, features(featureSusPay)); auto T = [&env](NetClock::duration const& d) { return env.now() + d; }; env.fund(XRP(5000), "alice", "bob"); @@ -222,7 +228,7 @@ struct SusPay_test : public beast::unit_test::suite using namespace std::chrono; using S = seconds; { - Env env(*this); + Env env(*this, features(featureSusPay)); auto T = [&env](NetClock::duration const& d) { return env.now() + d; }; env.fund(XRP(5000), "alice", "bob"); @@ -244,7 +250,7 @@ struct SusPay_test : public beast::unit_test::suite using namespace std::chrono; using S = seconds; { - Env env(*this); + Env env(*this, features(featureSusPay)); auto T = [&env](NetClock::duration const& d) { return env.now() + d; }; env.fund(XRP(5000), "alice", "bob", "carol"); @@ -270,7 +276,7 @@ struct SusPay_test : public beast::unit_test::suite env.close(); } { - Env env(*this); + Env env(*this, features(featureSusPay)); auto T = [&env](NetClock::duration const& d) { return env.now() + d; }; env.fund(XRP(5000), "alice", "bob", "carol"); @@ -287,7 +293,7 @@ struct SusPay_test : public beast::unit_test::suite expect(! env.le(keylet::susPay(Account("alice").id(), seq))); } { - Env env(*this); + Env env(*this, features(featureSusPay)); auto T = [&env](NetClock::duration const& d) { return env.now() + d; }; env.fund(XRP(5000), "alice", "bob", "carol"); @@ -306,7 +312,7 @@ struct SusPay_test : public beast::unit_test::suite env.require(balance("carol", XRP(5000))); } { - Env env(*this); + Env env(*this, features(featureSusPay)); auto T = [&env](NetClock::duration const& d) { return env.now() + d; }; env.fund(XRP(5000), "alice", "bob", "carol"); @@ -318,7 +324,7 @@ struct SusPay_test : public beast::unit_test::suite env(finish("bob", "alice", seq, cx.first, cx.second), ter(tecNO_PERMISSION)); } { - Env env(*this); + Env env(*this, features(featureSusPay)); auto T = [&env](NetClock::duration const& d) { return env.now() + d; }; env.fund(XRP(5000), "alice", "bob", "carol"); @@ -338,7 +344,7 @@ struct SusPay_test : public beast::unit_test::suite using namespace jtx; using namespace std::chrono; using S = seconds; - Env env(*this); + Env env(*this, features(featureSusPay)); auto T = [&env](NetClock::duration const& d) { return env.now() + d; }; env.fund(XRP(5000), "alice", "bob", "carol"); diff --git a/src/ripple/app/tx/impl/CancelTicket.cpp b/src/ripple/app/tx/impl/CancelTicket.cpp index 84844b5d71..78e07bc10f 100644 --- a/src/ripple/app/tx/impl/CancelTicket.cpp +++ b/src/ripple/app/tx/impl/CancelTicket.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -28,10 +29,9 @@ namespace ripple { TER CancelTicket::preflight (PreflightContext const& ctx) { -#if ! RIPPLE_ENABLE_TICKETS - if (! (ctx.flags & tapENABLE_TESTING)) + if (! ctx.rules.enabled(featureTickets, + ctx.app.config().features)) return temDISABLED; -#endif auto const ret = preflight1 (ctx); if (!isTesSuccess (ret)) diff --git a/src/ripple/app/tx/impl/CreateTicket.cpp b/src/ripple/app/tx/impl/CreateTicket.cpp index 271cc29476..5ba86a4727 100644 --- a/src/ripple/app/tx/impl/CreateTicket.cpp +++ b/src/ripple/app/tx/impl/CreateTicket.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include namespace ripple { @@ -28,10 +29,9 @@ namespace ripple { TER CreateTicket::preflight (PreflightContext const& ctx) { -#if ! RIPPLE_ENABLE_TICKETS - if (! (ctx.flags & tapENABLE_TESTING)) + if (! ctx.rules.enabled(featureTickets, + ctx.app.config().features)) return temDISABLED; -#endif auto const ret = preflight1 (ctx); if (!isTesSuccess (ret)) diff --git a/src/ripple/app/tx/impl/SetAccount.cpp b/src/ripple/app/tx/impl/SetAccount.cpp index aab888b92c..5e9373586a 100644 --- a/src/ripple/app/tx/impl/SetAccount.cpp +++ b/src/ripple/app/tx/impl/SetAccount.cpp @@ -253,9 +253,8 @@ SetAccount::doApply () // Account has no regular key or multi-signer signer list. // Prevent transaction changes until we're ready. - if (view().flags() & tapENABLE_TESTING || - view().rules().enabled(featureMultiSign, - ctx_.app.config().features)) + if (view().rules().enabled(featureMultiSign, + ctx_.app.config().features)) return tecNO_ALTERNATIVE_KEY; return tecNO_REGULAR_KEY; diff --git a/src/ripple/app/tx/impl/SetSignerList.cpp b/src/ripple/app/tx/impl/SetSignerList.cpp index 040345b801..eea1d5dcea 100644 --- a/src/ripple/app/tx/impl/SetSignerList.cpp +++ b/src/ripple/app/tx/impl/SetSignerList.cpp @@ -73,9 +73,8 @@ SetSignerList::determineOperation(STTx const& tx, TER SetSignerList::preflight (PreflightContext const& ctx) { - if (! (ctx.flags & tapENABLE_TESTING) && - ! ctx.rules.enabled(featureMultiSign, - ctx.app.config().features)) + if (! ctx.rules.enabled(featureMultiSign, + ctx.app.config().features)) return temDISABLED; auto const ret = preflight1 (ctx); diff --git a/src/ripple/app/tx/impl/SetTrust.cpp b/src/ripple/app/tx/impl/SetTrust.cpp index 85b8244cfa..85b770c82e 100644 --- a/src/ripple/app/tx/impl/SetTrust.cpp +++ b/src/ripple/app/tx/impl/SetTrust.cpp @@ -430,9 +430,7 @@ SetTrust::doApply () else if (! saLimitAmount && // Setting default limit. (! bQualityIn || ! uQualityIn) && // Not setting quality in or setting default quality in. (! bQualityOut || ! uQualityOut) && // Not setting quality out or setting default quality out. - (! ((view().flags() & tapENABLE_TESTING) || - view().rules().enabled(featureTrustSetAuth, - ctx_.app.config().features)) || ! bSetAuth)) + (! (view().rules().enabled(featureTrustSetAuth, ctx_.app.config().features)) || ! bSetAuth)) { j_.trace << "Redundant: Setting non-existent ripple line to defaults."; diff --git a/src/ripple/app/tx/impl/SusPay.cpp b/src/ripple/app/tx/impl/SusPay.cpp index 6fb16bc0f4..d1b121d62c 100644 --- a/src/ripple/app/tx/impl/SusPay.cpp +++ b/src/ripple/app/tx/impl/SusPay.cpp @@ -131,8 +131,7 @@ namespace ripple { TER SusPayCreate::preflight (PreflightContext const& ctx) { - if (! (ctx.flags & tapENABLE_TESTING) && - ! ctx.rules.enabled(featureSusPay, + if (! ctx.rules.enabled(featureSusPay, ctx.app.config().features)) return temDISABLED; @@ -251,8 +250,7 @@ SusPayCreate::doApply() TER SusPayFinish::preflight (PreflightContext const& ctx) { - if (! (ctx.flags & tapENABLE_TESTING) && - ! ctx.rules.enabled(featureSusPay, + if (! ctx.rules.enabled(featureSusPay, ctx.app.config().features)) return temDISABLED; @@ -371,8 +369,7 @@ SusPayFinish::doApply() TER SusPayCancel::preflight (PreflightContext const& ctx) { - if (! (ctx.flags & tapENABLE_TESTING) && - ! ctx.rules.enabled(featureSusPay, + if (! ctx.rules.enabled(featureSusPay, ctx.app.config().features)) return temDISABLED; diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index 85d2b58da9..d5564d59ce 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -314,8 +314,7 @@ TER Transactor::checkSign (PreclaimContext const& ctx) { // Make sure multisigning is enabled before we check for multisignatures. - if ((ctx.flags & tapENABLE_TESTING) || - (ctx.view.rules().enabled(featureMultiSign, + if ((ctx.view.rules().enabled(featureMultiSign, ctx.app.config().features))) { auto pk = diff --git a/src/ripple/app/tx/impl/apply.cpp b/src/ripple/app/tx/impl/apply.cpp index a00a15e1f9..50f2a3f546 100644 --- a/src/ripple/app/tx/impl/apply.cpp +++ b/src/ripple/app/tx/impl/apply.cpp @@ -83,9 +83,8 @@ checkValidity(HashRouter& router, ApplyFlags const& flags) { auto const allowMultiSign = - (flags & tapENABLE_TESTING) || - (rules.enabled(featureMultiSign, - config.features)); + rules.enabled(featureMultiSign, + config.features); return checkValidity(router, tx, allowMultiSign); } diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 763fdfaa4b..e7ced1b0eb 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -35,6 +35,7 @@ feature (const char* name); /** @} */ extern uint256 const featureMultiSign; +extern uint256 const featureTickets; extern uint256 const featureSusPay; extern uint256 const featureTrustSetAuth; extern uint256 const featureFeeEscalation; diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 1cdc5cba34..9fd8a37688 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -46,8 +46,8 @@ feature (const char* name) } uint256 const featureMultiSign = feature("MultiSign"); +uint256 const featureTickets = feature("Tickets"); uint256 const featureSusPay = feature("SusPay"); uint256 const featureTrustSetAuth = feature("TrustSetAuth"); uint256 const featureFeeEscalation = feature("FeeEscalation"); - } // ripple diff --git a/src/ripple/rpc/tests/JSONRPC.test.cpp b/src/ripple/rpc/tests/JSONRPC.test.cpp index 8e9a389d34..2fa2c359d3 100644 --- a/src/ripple/rpc/tests/JSONRPC.test.cpp +++ b/src/ripple/rpc/tests/JSONRPC.test.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -1648,7 +1649,7 @@ public: // "b" (not in the ledger) is rDg53Haik2475DJx8bjMDSDPj4VX7htaMd. // "c" (phantom signer) is rPcNzota6B8YBokhYtcTNqQVCngtbnWfux. - test::jtx::Env env(*this); + test::jtx::Env env(*this, test::jtx::features(featureMultiSign)); env.fund(test::jtx::XRP(100000), a, g); env.close(); diff --git a/src/ripple/test/jtx/impl/Env_test.cpp b/src/ripple/test/jtx/impl/Env_test.cpp index 8e99170758..f36056f5d0 100644 --- a/src/ripple/test/jtx/impl/Env_test.cpp +++ b/src/ripple/test/jtx/impl/Env_test.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -352,7 +353,7 @@ public: { using namespace jtx; - Env env(*this); + Env env(*this, features(featureMultiSign)); env.fund(XRP(10000), "alice"); env(signers("alice", 1, { { "alice", 1 }, { "bob", 2 } }), ter(temBAD_SIGNER)); @@ -381,14 +382,12 @@ public: ticket::create("alice", 60, "bob"); { - Env env(*this); + Env env(*this, features(featureTickets)); env.fund(XRP(10000), "alice"); env(noop("alice"), require(owners("alice", 0), tickets("alice", 0))); env(ticket::create("alice"), require(owners("alice", 1), tickets("alice", 1))); env(ticket::create("alice"), require(owners("alice", 2), tickets("alice", 2))); } - - Env env(*this); } struct UDT