From 026a2491735f45d54b0bfeec6a06fe4c6420f981 Mon Sep 17 00:00:00 2001 From: Mike Ellery Date: Thu, 23 Feb 2017 08:11:44 -0800 Subject: [PATCH] Implement transaction invariant checks (RIPD-1425): Add new functionality to enforce one or more sanity checks (invariants) on transactions. Add tests for each new invariant check. Allow for easily adding additional invariant checks in the future. Also Resolves ------------- - RIPD-1426 - RIPD-1427 - RIPD-1428 - RIPD-1429 - RIPD-1430 - RIPD-1431 - RIPD-1432 Release Notes ------------- Creates a new ammendment named "EnforceInvariants" which must be enabled in order for these new checks to run on each transaction. --- Builds/VisualStudio2015/RippleD.vcxproj | 10 + .../VisualStudio2015/RippleD.vcxproj.filters | 9 + docs/source.dox | 1 + src/ripple/app/main/Amendments.cpp | 3 +- src/ripple/app/tx/impl/ApplyContext.cpp | 54 ++++ src/ripple/app/tx/impl/ApplyContext.h | 6 + src/ripple/app/tx/impl/InvariantCheck.cpp | 191 ++++++++++++++ src/ripple/app/tx/impl/InvariantCheck.h | 204 +++++++++++++++ src/ripple/app/tx/impl/Transactor.cpp | 75 ++++-- src/ripple/app/tx/impl/Transactor.h | 1 + src/ripple/protocol/Feature.h | 1 + src/ripple/protocol/STLedgerEntry.h | 4 + src/ripple/protocol/TER.h | 4 +- src/ripple/protocol/impl/Feature.cpp | 1 + src/ripple/protocol/impl/TER.cpp | 4 +- src/ripple/unity/app_tx.cpp | 1 + src/test/jtx/Env.h | 7 +- src/test/ledger/Invariants_test.cpp | 236 ++++++++++++++++++ src/test/rpc/LedgerRPC_test.cpp | 16 +- src/test/unity/ledger_test_unity.cpp | 1 + 20 files changed, 789 insertions(+), 40 deletions(-) create mode 100644 src/ripple/app/tx/impl/InvariantCheck.cpp create mode 100644 src/ripple/app/tx/impl/InvariantCheck.h create mode 100644 src/test/ledger/Invariants_test.cpp diff --git a/Builds/VisualStudio2015/RippleD.vcxproj b/Builds/VisualStudio2015/RippleD.vcxproj index 18cb38e2eb..1ef62f4db2 100644 --- a/Builds/VisualStudio2015/RippleD.vcxproj +++ b/Builds/VisualStudio2015/RippleD.vcxproj @@ -1311,6 +1311,12 @@ + + True + True + + + @@ -4699,6 +4705,10 @@ True True + + True + True + True True diff --git a/Builds/VisualStudio2015/RippleD.vcxproj.filters b/Builds/VisualStudio2015/RippleD.vcxproj.filters index 097689aecb..b6b237c0b2 100644 --- a/Builds/VisualStudio2015/RippleD.vcxproj.filters +++ b/Builds/VisualStudio2015/RippleD.vcxproj.filters @@ -1833,6 +1833,12 @@ ripple\app\tx\impl + + ripple\app\tx\impl + + + ripple\app\tx\impl + ripple\app\tx\impl @@ -5448,6 +5454,9 @@ test\ledger + + test\ledger + test\ledger diff --git a/docs/source.dox b/docs/source.dox index 5c5affb668..6dd9f77a67 100644 --- a/docs/source.dox +++ b/docs/source.dox @@ -117,6 +117,7 @@ INPUT = \ ../src/ripple/app/consensus/RCLCxLedger.h \ ../src/ripple/app/consensus/RCLConsensus.h \ ../src/ripple/app/consensus/RCLCxPeerPos.h \ + ../src/ripple/app/tx/impl/InvariantCheck.h \ INPUT_ENCODING = UTF-8 FILE_PATTERNS = diff --git a/src/ripple/app/main/Amendments.cpp b/src/ripple/app/main/Amendments.cpp index d470eaf61d..11e774d92c 100644 --- a/src/ripple/app/main/Amendments.cpp +++ b/src/ripple/app/main/Amendments.cpp @@ -52,7 +52,8 @@ supportedAmendments () { "E2E6F2866106419B88C50045ACE96368558C345566AC8F2BDF5A5B5587F0E6FA fix1368" }, { "07D43DCE529B15A10827E5E04943B496762F9A88E3268269D69C44BE49E21104 Escrow" }, { "86E83A7D2ECE3AD5FA87AB2195AE015C950469ABF0B72EAACED318F74886AE90 CryptoConditionsSuite" }, - { "48C4451D6C6A138453F056EB6793AFF4B5C57457A37BA63EF3541FF8CE873DC2 ToStrandV2"} + { "48C4451D6C6A138453F056EB6793AFF4B5C57457A37BA63EF3541FF8CE873DC2 ToStrandV2"}, + { "DC9CA96AEA1DCF83E527D1AFC916EFAF5D27388ECA4060A88817C1238CAEE0BF EnforceInvariants" } }; } diff --git a/src/ripple/app/tx/impl/ApplyContext.cpp b/src/ripple/app/tx/impl/ApplyContext.cpp index f18e90fa7f..31ec117998 100644 --- a/src/ripple/app/tx/impl/ApplyContext.cpp +++ b/src/ripple/app/tx/impl/ApplyContext.cpp @@ -19,10 +19,12 @@ #include #include +#include #include #include #include #include +#include #include namespace ripple { @@ -69,4 +71,56 @@ ApplyContext::visit (std::function visit(base_, func); } +template +TER +ApplyContext::checkInvariantsHelper(TER terResult, std::index_sequence) +{ + if (view_->rules().enabled(featureEnforceInvariants)) + { + auto checkers = getInvariantChecks(); + + // call each check's per-entry method + visit ( + [&checkers]( + uint256 const& index, + bool isDelete, + std::shared_ptr const& before, + std::shared_ptr const& after) + { + // Sean Parent for_each_argument trick + (void)std::array{ + {((std::get(checkers). + visitEntry(index, isDelete, before, after)), 0)...} + }; + }); + + // Sean Parent for_each_argument trick + // (a fold expression with `&&` would be really nice here when we move + // to C++-17) + std::array finalizers {{ + std::get(checkers).finalize(tx, terResult, journal)...}}; + + // call each check's finalizer to see that it passes + if (! std::all_of( finalizers.cbegin(), finalizers.cend(), + [](auto const& b) { return b; })) + { + terResult = (terResult == tecINVARIANT_FAILED) ? + tefINVARIANT_FAILED : + tecINVARIANT_FAILED ; + JLOG(journal.error()) << + "Transaction has failed one or more invariants: " << + to_string(tx.getJson (0)); + } + } + + return terResult; +} + +TER +ApplyContext::checkInvariants(TER terResult) +{ + return checkInvariantsHelper( + terResult, std::make_index_sequence::value>{}); +} + } // ripple diff --git a/src/ripple/app/tx/impl/ApplyContext.h b/src/ripple/app/tx/impl/ApplyContext.h index 95e7db829f..a4cfb8cf55 100644 --- a/src/ripple/app/tx/impl/ApplyContext.h +++ b/src/ripple/app/tx/impl/ApplyContext.h @@ -101,7 +101,13 @@ public: view_->rawDestroyXRP(fee); } + TER + checkInvariants(TER); + private: + template + TER checkInvariantsHelper(TER terResult, std::index_sequence); + OpenView& base_; ApplyFlags flags_; boost::optional view_; diff --git a/src/ripple/app/tx/impl/InvariantCheck.cpp b/src/ripple/app/tx/impl/InvariantCheck.cpp new file mode 100644 index 0000000000..03a15c5762 --- /dev/null +++ b/src/ripple/app/tx/impl/InvariantCheck.cpp @@ -0,0 +1,191 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2012-2016 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 + +namespace ripple { + +void +XRPNotCreated::visitEntry( + uint256 const&, + bool isDelete, + std::shared_ptr const& before, + std::shared_ptr const& after) +{ + if(before) + { + switch (before->getType()) + { + case ltACCOUNT_ROOT: + drops_ -= (*before)[sfBalance].xrp().drops(); + break; + case ltPAYCHAN: + drops_ -= ((*before)[sfAmount] - (*before)[sfBalance]).xrp().drops(); + break; + case ltESCROW: + drops_ -= (*before)[sfAmount].xrp().drops(); + break; + default: + break; + } + } + + if(after) + { + switch (after->getType()) + { + case ltACCOUNT_ROOT: + drops_ += (*after)[sfBalance].xrp().drops(); + break; + case ltPAYCHAN: + if (! isDelete) + drops_ += ((*after)[sfAmount] - (*after)[sfBalance]).xrp().drops(); + break; + case ltESCROW: + if (! isDelete) + drops_ += (*after)[sfAmount].xrp().drops(); + break; + default: + break; + } + } +} + +bool +XRPNotCreated::finalize(STTx const& tx, TER /*tec*/, beast::Journal const& j) +{ + auto fee = tx.getFieldAmount(sfFee).xrp().drops(); + if(-1*fee <= drops_ && drops_ <= 0) + return true; + + JLOG(j.fatal()) << "Invariant failed: XRP net change was " << drops_ << + " on a fee of " << fee; + return false; +} + +//------------------------------------------------------------------------------ + +void +AccountRootsNotDeleted::visitEntry( + uint256 const&, + bool isDelete, + std::shared_ptr const& before, + std::shared_ptr const&) +{ + if (isDelete && before && before->getType() == ltACCOUNT_ROOT) + accountDeleted_ = true; +} + +bool +AccountRootsNotDeleted::finalize(STTx const&, TER, beast::Journal const& j) +{ + if (! accountDeleted_) + return true; + + JLOG(j.fatal()) << "Invariant failed: an account root was deleted"; + return false; +} + +//------------------------------------------------------------------------------ + +void +LedgerEntryTypesMatch::visitEntry( + uint256 const&, + bool, + std::shared_ptr const& before, + std::shared_ptr const& after) +{ + if (before && after && before->getType() != after->getType()) + typeMismatch_ = true; + + if (after) + { + switch (after->getType()) + { + case ltACCOUNT_ROOT: + case ltDIR_NODE: + case ltRIPPLE_STATE: + case ltTICKET: + case ltSIGNER_LIST: + case ltOFFER: + case ltLEDGER_HASHES: + case ltAMENDMENTS: + case ltFEE_SETTINGS: + case ltESCROW: + case ltPAYCHAN: + break; + default: + invalidTypeAdded_ = true; + break; + } + } +} + +bool +LedgerEntryTypesMatch::finalize(STTx const&, TER, beast::Journal const& j) +{ + if ((! typeMismatch_) && (! invalidTypeAdded_)) + return true; + + if (typeMismatch_) + { + JLOG(j.fatal()) << "Invariant failed: ledger entry type mismatch"; + } + + if (invalidTypeAdded_) + { + JLOG(j.fatal()) << "Invariant failed: invalid ledger entry type added"; + } + + return false; +} + +//------------------------------------------------------------------------------ + +void +NoXRPTrustLines::visitEntry( + uint256 const&, + bool, + std::shared_ptr const&, + std::shared_ptr const& after) +{ + if (after && after->getType() == ltRIPPLE_STATE) + { + // checking the issue directly here instead of + // relying on .native() just in case native somehow + // were systematically incorrect + xrpTrustLine_ = + after->getFieldAmount (sfLowLimit).issue() == xrpIssue() || + after->getFieldAmount (sfHighLimit).issue() == xrpIssue(); + } +} + +bool +NoXRPTrustLines::finalize(STTx const&, TER, beast::Journal const& j) +{ + if (! xrpTrustLine_) + return true; + + JLOG(j.fatal()) << "Invariant failed: an XRP trust line was created"; + return false; +} + +} // ripple + diff --git a/src/ripple/app/tx/impl/InvariantCheck.h b/src/ripple/app/tx/impl/InvariantCheck.h new file mode 100644 index 0000000000..554bb9cf27 --- /dev/null +++ b/src/ripple/app/tx/impl/InvariantCheck.h @@ -0,0 +1,204 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2012-2017 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. +*/ +//============================================================================== + +#ifndef RIPPLE_APP_TX_INVARIANTCHECK_H_INCLUDED +#define RIPPLE_APP_TX_INVARIANTCHECK_H_INCLUDED + +#include +#include +#include +#include +#include +#include +#include + +namespace ripple { + +#if GENERATING_DOCS +/** + * @brief Prototype for invariant check implementations. + * + * __THIS CLASS DOES NOT EXIST__ - or rather it exists in documentation only to + * communicate the interface required of any invariant checker. Any invariant + * check implementation should implement the public methods documented here. + * + */ +class InvariantChecker_PROTOTYPE +{ +public: + + /** + * @brief called for each ledger entry in the current transaction. + * + * @param index the key (identifier) for the ledger entry + * @param isDelete true if the SLE is being deleted + * @param before ledger entry before modification by the transaction + * @param after ledger entry after modification by the transaction + */ + void + visitEntry( + uint256 const& index, + bool isDelete, + std::shared_ptr const& before, + std::shared_ptr const& after); + + /** + * @brief called after all ledger entries have been visited to determine + * the final status of the check + * + * @param tx the transaction being applied + * @param tec the current TER result of the transaction + * @param j journal for logging + * + * @return true if check passes, false if it fails + */ + bool + finalize( + STTx const& tx, + TER tec, + beast::Journal const& j); +}; +#endif + +/** + * @brief Invariant: A transaction must not create XRP and should only destroy + * XRP, up to the transaction fee. + * + * For this check, we start with a signed 64-bit integer set to zero. As we go + * through the ledger entries, look only at account roots, escrow payments, + * and payment channels. Remove from the total any previous XRP values and add + * to the total any new XRP values. The net balance of a payment channel is + * computed from two fields (amount and balance) and deletions are ignored + * for paychan and escrow because the amount fields have not been adjusted for + * those in the case of deletion. + * + * The final total must be less than or equal to zero and greater than or equal + * to the negative of the tx fee. + * + */ +class XRPNotCreated +{ + std::int64_t drops_ = 0; + +public: + + void + visitEntry( + uint256 const&, + bool, + std::shared_ptr const&, + std::shared_ptr const&); + + bool + finalize(STTx const&, TER, beast::Journal const&); +}; + +/** + * @brief Invariant: we cannot remove an account ledger entry + * + * an account root should never be the target of a delete + */ +class AccountRootsNotDeleted +{ + bool accountDeleted_ = false; + +public: + + void + visitEntry( + uint256 const&, + bool, + std::shared_ptr const&, + std::shared_ptr const&); + + bool + finalize(STTx const&, TER, beast::Journal const&); +}; + +/** + * @brief Invariant: corresponding modified ledger entries should match in type and + * added entries should be a valid type. + * + */ +class LedgerEntryTypesMatch +{ + bool typeMismatch_ = false; + bool invalidTypeAdded_ = false; + +public: + + void + visitEntry( + uint256 const&, + bool, + std::shared_ptr const&, + std::shared_ptr const&); + + bool + finalize(STTx const&, TER, beast::Journal const&); +}; + +/** + * @brief Invariant: Trust lines using XRP are not allowed. + * + */ +class NoXRPTrustLines +{ + bool xrpTrustLine_ = false; + +public: + + void + visitEntry( + uint256 const&, + bool, + std::shared_ptr const&, + std::shared_ptr const&); + + bool + finalize(STTx const&, TER, beast::Journal const&); +}; + +// additional invariant checks can be declared above and then added to this +// tuple +using InvariantChecks = std::tuple< + AccountRootsNotDeleted, + LedgerEntryTypesMatch, + XRPNotCreated, + NoXRPTrustLines +>; + +/** + * @brief get a tuple of all invariant checks + * + * @return std::tuple of instances that implement the required invariant check + * methods + * + * @see ripple::InvariantChecker_PROTOTYPE + */ +inline +InvariantChecks +getInvariantChecks() +{ + return InvariantChecks{}; +} + +} //ripple + +#endif diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index 7b62b305f4..1cf50da679 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -568,6 +568,33 @@ void removeUnfundedOffers (ApplyView& view, std::vector const& offers, } } +void +Transactor::claimFee (XRPAmount& fee, TER terResult, std::vector const& removedOffers) +{ + ctx_.discard(); + + auto const txnAcct = view().peek( + keylet::account(ctx_.tx.getAccountID(sfAccount))); + + auto const balance = txnAcct->getFieldAmount (sfBalance).xrp (); + + // balance should have already been + // checked in checkFee / preFlight. + assert(balance != zero && (!view().open() || balance >= fee)); + // We retry/reject the transaction if the account + // balance is zero or we're applying against an open + // ledger and the balance is less than the fee + if (fee > balance) + fee = balance; + txnAcct->setFieldAmount (sfBalance, balance - fee); + txnAcct->setFieldU32 (sfSequence, ctx_.tx.getSequence() + 1); + + if (terResult == tecOVERSIZE) + removeUnfundedOffers (view(), removedOffers, ctx_.app.journal ("View")); + + view().update (txnAcct); +} + //------------------------------------------------------------------------------ std::pair Transactor::operator()() @@ -655,35 +682,24 @@ Transactor::operator()() }); } - ctx_.discard(); - - auto const txnAcct = view().peek( - keylet::account(ctx_.tx.getAccountID(sfAccount))); - - std::uint32_t t_seq = ctx_.tx.getSequence (); - - auto const balance = txnAcct->getFieldAmount (sfBalance).xrp (); - - // balance should have already been - // checked in checkFee / preFlight. - assert(balance != zero && (!view().open() || balance >= fee)); - // We retry/reject the transaction if the account - // balance is zero or we're applying against an open - // ledger and the balance is less than the fee - if (fee > balance) - fee = balance; - txnAcct->setFieldAmount (sfBalance, balance - fee); - txnAcct->setFieldU32 (sfSequence, t_seq + 1); - - if (terResult == tecOVERSIZE) - removeUnfundedOffers (view(), removedOffers, ctx_.app.journal ("View")); - - view().update (txnAcct); + claimFee(fee, terResult, removedOffers); didApply = true; } - else if (!didApply) + + if (didApply) { - JLOG(j_.debug()) << "Not applying transaction " << txID; + // Check invariants + // if `tecINVARIANT_FAILED` not returned, we can proceed to apply the tx + terResult = ctx_.checkInvariants(terResult); + if (terResult == tecINVARIANT_FAILED) + { + // if invariants failed, claim a fee still + claimFee(fee, terResult, {}); + //Check invariants *again* to ensure the fee claiming doesn't + //violate invariants. + terResult = ctx_.checkInvariants(terResult); + didApply = isTecClaim(terResult); + } } if (didApply) @@ -691,7 +707,7 @@ Transactor::operator()() // Transaction succeeded fully or (retries are // not allowed and the transaction could claim a fee) - if(!view().open()) + if (!view().open()) { // Charge whatever fee they specified. @@ -711,6 +727,11 @@ Transactor::operator()() // since we called apply(), it is not okay to look // at view() past this point. } + else + { + JLOG(j_.debug()) << "Not applying transaction " << txID; + } + JLOG(j_.trace()) << "apply: " << transToken(terResult) << diff --git a/src/ripple/app/tx/impl/Transactor.h b/src/ripple/app/tx/impl/Transactor.h index bb878539c6..cf74ee0a99 100644 --- a/src/ripple/app/tx/impl/Transactor.h +++ b/src/ripple/app/tx/impl/Transactor.h @@ -168,6 +168,7 @@ protected: private: void setSeq (); TER payFee (); + void claimFee (XRPAmount& fee, TER terResult, std::vector const& removedOffers); static TER checkSingleSign (PreclaimContext const& ctx); static TER checkMultiSign (PreclaimContext const& ctx); }; diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 352040295e..89f4de4d50 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -49,6 +49,7 @@ extern uint256 const fix1368; extern uint256 const featureEscrow; extern uint256 const featureCryptoConditionsSuite; extern uint256 const featureToStrandV2; +extern uint256 const featureEnforceInvariants; } // ripple diff --git a/src/ripple/protocol/STLedgerEntry.h b/src/ripple/protocol/STLedgerEntry.h index 24a85f407d..83ffdc5591 100644 --- a/src/ripple/protocol/STLedgerEntry.h +++ b/src/ripple/protocol/STLedgerEntry.h @@ -25,10 +25,14 @@ namespace ripple { +class Invariants_test; + class STLedgerEntry final : public STObject , public CountedObject { + friend Invariants_test; // this test wants access to the private type_ + public: static char const* getCountedObjectName () { return "STLedgerEntry"; } diff --git a/src/ripple/protocol/TER.h b/src/ripple/protocol/TER.h index 37b8938113..811de43e9a 100644 --- a/src/ripple/protocol/TER.h +++ b/src/ripple/protocol/TER.h @@ -120,6 +120,7 @@ enum TER tefBAD_QUORUM, tefNOT_MULTI_SIGNING, tefBAD_AUTH_MASTER, + tefINVARIANT_FAILED, // -99 .. -1: R Retry // sequence too high, no funds for txn fee, originating -account @@ -206,7 +207,8 @@ enum TER tecDST_TAG_NEEDED = 143, tecINTERNAL = 144, tecOVERSIZE = 145, - tecCRYPTOCONDITION_ERROR = 146 + tecCRYPTOCONDITION_ERROR = 146, + tecINVARIANT_FAILED = 147 }; inline bool isTelLocal(TER x) diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index e485daf3fb..4801267938 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -60,5 +60,6 @@ uint256 const fix1368 = feature("fix1368"); uint256 const featureEscrow = feature("Escrow"); uint256 const featureCryptoConditionsSuite = feature("CryptoConditionsSuite"); uint256 const featureToStrandV2 = feature("ToStrandV2"); +uint256 const featureEnforceInvariants = feature("EnforceInvariants"); } // ripple diff --git a/src/ripple/protocol/impl/TER.cpp b/src/ripple/protocol/impl/TER.cpp index fede93f6e7..b4674923e3 100644 --- a/src/ripple/protocol/impl/TER.cpp +++ b/src/ripple/protocol/impl/TER.cpp @@ -64,6 +64,7 @@ bool transResultInfo (TER code, std::string& token, std::string& text) { tecDST_TAG_NEEDED, { "tecDST_TAG_NEEDED", "A destination tag is required." } }, { tecINTERNAL, { "tecINTERNAL", "An internal error has occurred during processing." } }, { tecCRYPTOCONDITION_ERROR, { "tecCRYPTOCONDITION_ERROR", "Malformed, invalid, or mismatched conditional or fulfillment." } }, + { tecINVARIANT_FAILED, { "tecINVARIANT_FAILED", "One or more invariants for the transaction were not satisfied." } }, { tefALREADY, { "tefALREADY", "The exact transaction was already in this ledger." } }, { tefBAD_ADD_AUTH, { "tefBAD_ADD_AUTH", "Not authorized to add account." } }, @@ -79,9 +80,10 @@ bool transResultInfo (TER code, std::string& token, std::string& text) { tefMAX_LEDGER, { "tefMAX_LEDGER", "Ledger sequence too high." } }, { tefNO_AUTH_REQUIRED, { "tefNO_AUTH_REQUIRED", "Auth is not required." } }, { tefNOT_MULTI_SIGNING, { "tefNOT_MULTI_SIGNING", "Account has no appropriate list of multi-signers." } }, - { tefPAST_SEQ, { "tefPAST_SEQ", "This sequence number has already past." } }, + { tefPAST_SEQ, { "tefPAST_SEQ", "This sequence number has already passed." } }, { tefWRONG_PRIOR, { "tefWRONG_PRIOR", "This previous transaction does not match." } }, { tefBAD_AUTH_MASTER, { "tefBAD_AUTH_MASTER", "Auth for unclaimed account needs correct master key." } }, + { tefINVARIANT_FAILED, { "tefINVARIANT_FAILED", "Fee claim violated invariants for the transaction." } }, { telLOCAL_ERROR, { "telLOCAL_ERROR", "Local failure." } }, { telBAD_DOMAIN, { "telBAD_DOMAIN", "Domain too long." } }, diff --git a/src/ripple/unity/app_tx.cpp b/src/ripple/unity/app_tx.cpp index d1a669efa2..a676fb5f31 100644 --- a/src/ripple/unity/app_tx.cpp +++ b/src/ripple/unity/app_tx.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include diff --git a/src/test/jtx/Env.h b/src/test/jtx/Env.h index f7db2bbaf5..2f7d3ac54b 100644 --- a/src/test/jtx/Env.h +++ b/src/test/jtx/Env.h @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -53,6 +54,7 @@ #include #include + namespace ripple { namespace test { namespace jtx { @@ -156,7 +158,10 @@ public: { memoize(Account::master); Pathfinder::initPathTable(); - construct(std::forward(args)...); + // enable the the invariant enforcement amendment by default. + construct( + features(featureEnforceInvariants), + std::forward(args)...); } template diff --git a/src/test/ledger/Invariants_test.cpp b/src/test/ledger/Invariants_test.cpp new file mode 100644 index 0000000000..520d915a99 --- /dev/null +++ b/src/test/ledger/Invariants_test.cpp @@ -0,0 +1,236 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2012-2017 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 + +namespace ripple { + +class Invariants_test : public beast::unit_test::suite +{ + + class TestSink : public beast::Journal::Sink + { + public: + std::stringstream strm_; + + TestSink () : Sink (beast::severities::kWarning, false) { } + + void + write (beast::severities::Severity level, + std::string const& text) override + { + if (level < threshold()) + return; + + strm_ << text << std::endl; + } + }; + + // this is common setup/method for running a failing invariant check. The + // precheck function is used to manipulate the ApplyContext with view + // changes that will cause the check to fail. + void + doInvariantCheck( bool enabled, + std::function < + bool ( + test::jtx::Account const& a, + test::jtx::Account const& b, + ApplyContext& ac)> + const& precheck ) + { + using namespace test::jtx; + Env env {*this}; + if (! enabled) + { + auto& features = env.app().config().features; + auto it = features.find(featureEnforceInvariants); + if (it != features.end()) + features.erase(it); + } + + Account A1 {"A1"}; + Account A2 {"A2"}; + env.fund (XRP (1000), A1, A2); + env.close(); + + // dummy/empty tx to setup the AccountContext + auto tx = STTx {ttACCOUNT_SET, [](STObject&){ } }; + OpenView ov {*env.current()}; + TestSink sink; + beast::Journal jlog {sink}; + ApplyContext ac { + env.app(), + ov, + tx, + tesSUCCESS, + env.current()->fees().base, + tapNONE, + jlog + }; + + BEAST_EXPECT(precheck(A1, A2, ac)); + + auto tr = ac.checkInvariants(tesSUCCESS); + if (enabled) + { + BEAST_EXPECT(tr == tecINVARIANT_FAILED); + BEAST_EXPECT(boost::starts_with(sink.strm_.str(), "Invariant failed:")); + //uncomment if you want to log the invariant failure message + //log << " --> " << sink.strm_.str() << std::endl; + } + else + { + BEAST_EXPECT(tr == tesSUCCESS); + BEAST_EXPECT(sink.strm_.str().empty()); + } + } + + void + testEnabled () + { + using namespace test::jtx; + testcase ("feature enabled"); + Env env {*this}; + + auto hasInvariants = + env.app().config().features.find (featureEnforceInvariants); + BEAST_EXPECT(hasInvariants != env.app().config().features.end()); + + BEAST_EXPECT(env.current()->rules().enabled(featureEnforceInvariants)); + } + + void + testXRPNotCreated (bool enabled) + { + using namespace test::jtx; + testcase << "checks " << (enabled ? "enabled" : "disabled") << + " - XRP created"; + doInvariantCheck (enabled, + [](Account const& A1, Account const&, ApplyContext& ac) + { + // put a single account in the view and "manufacture" some XRP + auto const sle = ac.view().peek (keylet::account(A1.id())); + if(! sle) + return false; + auto amt = sle->getFieldAmount (sfBalance); + sle->setFieldAmount (sfBalance, amt + 500); + ac.view().update (sle); + return true; + }); + } + + void + testAccountsNotRemoved(bool enabled) + { + using namespace test::jtx; + testcase << "checks " << (enabled ? "enabled" : "disabled") << + " - account root removed"; + doInvariantCheck (enabled, + [](Account const& A1, Account const&, ApplyContext& ac) + { + // remove an account from the view + auto const sle = ac.view().peek (keylet::account(A1.id())); + if(! sle) + return false; + ac.view().erase (sle); + return true; + }); + } + + void + testTypesMatch(bool enabled) + { + using namespace test::jtx; + testcase << "checks " << (enabled ? "enabled" : "disabled") << + " - LE types don't match"; + doInvariantCheck (enabled, + [](Account const& A1, Account const&, ApplyContext& ac) + { + // replace an entry in the table with an SLE of a different type + auto const sle = ac.view().peek (keylet::account(A1.id())); + if(! sle) + return false; + auto sleNew = std::make_shared (ltTICKET, sle->key()); + ac.rawView().rawReplace (sleNew); + return true; + }); + + doInvariantCheck (enabled, + [](Account const& A1, Account const&, ApplyContext& ac) + { + // add an entry in the table with an SLE of an invalid type + auto const sle = ac.view().peek (keylet::account(A1.id())); + if(! sle) + return false; + // make a dummy escrow ledger entry, then change the type to an + // unsupported value so that the valid type invariant check + // will fail. + auto sleNew = std::make_shared ( + keylet::escrow(A1, (*sle)[sfSequence] + 2)); + sleNew->type_ = ltNICKNAME; + ac.view().insert (sleNew); + return true; + }); + } + + void + testNoXRPTrustLine(bool enabled) + { + using namespace test::jtx; + testcase << "checks " << (enabled ? "enabled" : "disabled") << + " - trust lines with XRP not allowed"; + doInvariantCheck (enabled, + [](Account const& A1, Account const& A2, ApplyContext& ac) + { + // create simple trust SLE with xrp currency + auto index = getRippleStateIndex (A1, A2, xrpIssue().currency); + auto const sleNew = std::make_shared( + ltRIPPLE_STATE, index); + ac.view().insert (sleNew); + return true; + }); + } + +public: + void run () + { + testEnabled (); + // all invariant checks are run with + // the checks enabled and disabled + for(auto const& b : {true, false}) + { + testXRPNotCreated (b); + testAccountsNotRemoved (b); + testTypesMatch (b); + testNoXRPTrustLine (b); + } + } +}; + +BEAST_DEFINE_TESTSUITE (Invariants, ledger, ripple); + +} // ripple diff --git a/src/test/rpc/LedgerRPC_test.cpp b/src/test/rpc/LedgerRPC_test.cpp index f6ecbd2140..596e463e25 100644 --- a/src/test/rpc/LedgerRPC_test.cpp +++ b/src/test/rpc/LedgerRPC_test.cpp @@ -470,15 +470,13 @@ class LedgerRPC_test : public beast::unit_test::suite { testcase("Ledger with Queued Transactions"); using namespace test::jtx; - Env env{ *this, []() - { - auto p = std::make_unique(); - test::setupConfigForUnitTests(*p); - auto& section = p->section("transaction_queue"); - section.set("minimum_txn_in_ledger_standalone", "3"); - return p; - }(), - features(featureFeeEscalation) }; + Env env { *this, + envconfig([](std::unique_ptr cfg) { + cfg->section("transaction_queue") + .set("minimum_txn_in_ledger_standalone", "3"); + return cfg; + }), + features(featureFeeEscalation)}; Json::Value jv; jv[jss::ledger_index] = "current"; diff --git a/src/test/unity/ledger_test_unity.cpp b/src/test/unity/ledger_test_unity.cpp index 9496f523fd..6338fa0a70 100644 --- a/src/test/unity/ledger_test_unity.cpp +++ b/src/test/unity/ledger_test_unity.cpp @@ -20,6 +20,7 @@ #include #include +#include #include #include #include