diff --git a/src/ripple/app/consensus/RCLValidations.cpp b/src/ripple/app/consensus/RCLValidations.cpp index c44f98bd48..6311677c21 100644 --- a/src/ripple/app/consensus/RCLValidations.cpp +++ b/src/ripple/app/consensus/RCLValidations.cpp @@ -182,13 +182,6 @@ handleNewValidation( << " [" << val->getSerializer().slice() << "]"; }; - if (!val->isFieldPresent(sfLedgerSequence)) - { - if (j.error()) - dmp(j.error(), "missing ledger sequence field"); - return false; - } - // masterKey is seated only if validator is trusted or listed if (masterKey) { diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 6b5446fb34..4518d61557 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -2170,7 +2170,8 @@ PeerImp::onMessage(std::shared_ptr const& m) } catch (std::exception const& e) { - JLOG(p_journal_.warn()) << "Validation: Exception, " << e.what(); + JLOG(p_journal_.warn()) + << "Exception processing validation: " << e.what(); fee_ = Resource::feeInvalidRequest; } } diff --git a/src/ripple/protocol/STValidation.h b/src/ripple/protocol/STValidation.h index 8410860007..996b56887d 100644 --- a/src/ripple/protocol/STValidation.h +++ b/src/ripple/protocol/STValidation.h @@ -122,6 +122,11 @@ public: // Perform additional initialization f(*this); + // This is a logic error because we should never assemble a + // validation without a ledger sequence number. + if (!isFieldPresent(sfLedgerSequence)) + LogicError("Missing ledger sequence in validation"); + // Finally, sign the validation and mark it as trusted: setFlag(vfFullyCanonicalSig); setFieldVL(sfSignature, signDigest(pk, sk, getSigningHash())); diff --git a/src/ripple/protocol/impl/STValidation.cpp b/src/ripple/protocol/impl/STValidation.cpp index 458b601bf7..779a031a7e 100644 --- a/src/ripple/protocol/impl/STValidation.cpp +++ b/src/ripple/protocol/impl/STValidation.cpp @@ -34,7 +34,7 @@ STValidation::validationFormat() static SOTemplate const format{ {sfFlags, soeREQUIRED}, {sfLedgerHash, soeREQUIRED}, - {sfLedgerSequence, soeOPTIONAL}, + {sfLedgerSequence, soeREQUIRED}, {sfCloseTime, soeOPTIONAL}, {sfLoadFee, soeOPTIONAL}, {sfAmendments, soeOPTIONAL}, diff --git a/src/test/app/RCLValidations_test.cpp b/src/test/app/RCLValidations_test.cpp index a84b44c301..6b4f16d42c 100644 --- a/src/test/app/RCLValidations_test.cpp +++ b/src/test/app/RCLValidations_test.cpp @@ -20,6 +20,8 @@ #include #include #include +#include +#include #include #include #include @@ -310,6 +312,89 @@ class RCLValidations_test : public beast::unit_test::suite BEAST_EXPECT(trie.branchSupport(ledg_259) == 4); } + void + testLedgerSequence() + { + testcase("Validations with and without the LedgerSequence field"); + + auto const nodeID = + from_hex_text("38ECC15DBD999DE4CE70A6DC69A4166AB18031A7"); + + try + { + std::string const withLedgerSequence = + "228000000126034B9FFF2926460DC55185937F7F41DD7977F21B9DF95FCB61" + "9E5132ABB0D7ADEA0F7CE8A9347871A34250179D85BDE824F57FFE0AC8F89B" + "55FCB89277272A1D83D08ADEC98096A88EF723137321029D19FB0940E5C0D8" + "5873FA711999944A687D129DA5C33E928C2751FC1B31EB3276463044022022" + "6229CF66A678EE021F62CA229BA006B41939845004D3FAF8347C6FFBB7C613" + "02200BE9CD3629FD67C6C672BD433A2769FCDB36B1ECA2292919C58A86224E" + "2BF5970313C13F00C1FC4A53E60AB02C864641002B3172F38677E29C26C540" + "6685179B37E1EDAC157D2D480E006395B76F948E3E07A45A05FE10230D88A7" + "993C71F97AE4B1F2D11F4AFA8FA1BC8827AD4C0F682C03A8B671DCDF6B5C4D" + "E36D44243A684103EF8825BA44241B3BD880770BFA4DA21C71805768318553" + "68CBEC6A3154FDE4A7676E3012E8230864E95A58C60FD61430D7E1B4D33531" + "95F2981DC12B0C7C0950FFAC30CD365592B8EE40489BA01AE2F7555CAC9C98" + "3145871DC82A42A31CF5BAE7D986E83A7D2ECE3AD5FA87AB2195AE015C9504" + "69ABF0B72EAACED318F74886AE9089308AF3B8B10B7192C4E613E1D2E4D9BA" + "64B2EE2D5232402AE82A6A7220D953"; + + if (auto ret = strUnHex(withLedgerSequence); ret) + { + SerialIter sit(makeSlice(*ret)); + + auto val = std::make_shared( + std::ref(sit), + [nodeID](PublicKey const& pk) { return nodeID; }, + false); + + BEAST_EXPECT(val); + BEAST_EXPECT(calcNodeID(val->getSignerPublic()) == nodeID); + BEAST_EXPECT(val->isFieldPresent(sfLedgerSequence)); + } + } + catch (std::exception const& ex) + { + fail(std::string("Unexpected exception thrown: ") + ex.what()); + } + + try + { + std::string const withoutLedgerSequence = + "22800000012926460DC55185937F7F41DD7977F21B9DF95FCB619E5132ABB0" + "D7ADEA0F7CE8A9347871A34250179D85BDE824F57FFE0AC8F89B55FCB89277" + "272A1D83D08ADEC98096A88EF723137321029D19FB0940E5C0D85873FA7119" + "99944A687D129DA5C33E928C2751FC1B31EB3276473045022100BE2EA49CF2" + "FFB7FE7A03F6860B8C35FEA04A064C7023FE28EC97E5A32E85DEC4022003B8" + "5D1D497F504B34F089D5BDB91BD888690C3D3A242A0FEF1DD52875FBA02E03" + "13C13F00C1FC4A53E60AB02C864641002B3172F38677E29C26C5406685179B" + "37E1EDAC157D2D480E006395B76F948E3E07A45A05FE10230D88A7993C71F9" + "7AE4B1F2D11F4AFA8FA1BC8827AD4C0F682C03A8B671DCDF6B5C4DE36D4424" + "3A684103EF8825BA44241B3BD880770BFA4DA21C7180576831855368CBEC6A" + "3154FDE4A7676E3012E8230864E95A58C60FD61430D7E1B4D3353195F2981D" + "C12B0C7C0950FFAC30CD365592B8EE40489BA01AE2F7555CAC9C983145871D" + "C82A42A31CF5BAE7D986E83A7D2ECE3AD5FA87AB2195AE015C950469ABF0B7" + "2EAACED318F74886AE9089308AF3B8B10B7192C4E613E1D2E4D9BA64B2EE2D" + "5232402AE82A6A7220D953"; + + if (auto ret = strUnHex(withoutLedgerSequence); ret) + { + SerialIter sit(makeSlice(*ret)); + + auto val = std::make_shared( + std::ref(sit), + [nodeID](PublicKey const& pk) { return nodeID; }, + false); + + fail("Expected exception not thrown from validation"); + } + } + catch (std::exception const& ex) + { + pass(); + } + } + public: void run() override @@ -317,6 +402,7 @@ public: testChangeTrusted(); testRCLValidatedLedger(); testLedgerTrieRCLValidatedLedger(); + testLedgerSequence(); } };