From 3af76ceb5a89f97656ece7059734b3bf3b9ab4bf Mon Sep 17 00:00:00 2001 From: Richard Holland Date: Mon, 19 Dec 2022 13:43:32 +0000 Subject: [PATCH 1/4] Add NetworkID field to Transaction common fields, enforced when network id > 1024 --- Builds/CMake/RippledCore.cmake | 1 + src/ripple/app/tx/impl/Transactor.cpp | 21 ++++ src/ripple/core/Config.h | 1 + src/ripple/core/ConfigSections.h | 1 + src/ripple/core/impl/Config.cpp | 12 ++ src/ripple/protocol/SField.h | 1 + src/ripple/protocol/TER.h | 3 + src/ripple/protocol/impl/SField.cpp | 1 + src/ripple/protocol/impl/TER.cpp | 3 + src/ripple/protocol/impl/TxFormats.cpp | 1 + src/ripple/protocol/jss.h | 1 + src/test/app/NetworkID_test.cpp | 153 +++++++++++++++++++++++++ src/test/core/Config_test.cpp | 66 +++++++++++ src/test/jtx/impl/Env.cpp | 3 + 14 files changed, 268 insertions(+) create mode 100644 src/test/app/NetworkID_test.cpp diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index 710ebb00aab..7d16f4c70c6 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -700,6 +700,7 @@ if (tests) src/test/app/LoadFeeTrack_test.cpp src/test/app/Manifest_test.cpp src/test/app/MultiSign_test.cpp + src/test/app/NetworkID_test.cpp src/test/app/NFToken_test.cpp src/test/app/NFTokenBurn_test.cpp src/test/app/NFTokenDir_test.cpp diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index 9265d365647..031f2d17d76 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -40,6 +40,27 @@ namespace ripple { NotTEC preflight0(PreflightContext const& ctx) { + uint32_t nodeNID = ctx.app.config().NETWORK_ID; + std::optional txNID = ctx.tx[~sfNetworkID]; + + if (nodeNID <= 1024) + { + // legacy networks have ids less than 1024, these networks cannot + // specify NetworkID in txn + if (txNID) + return telNETWORK_ID_MAKES_TX_NON_CANONICAL; + } + else + { + // new networks both require the field to be present and require it to + // match + if (!txNID) + return telREQUIRES_NETWORK_ID; + + if (*txNID != nodeNID) + return telWRONG_NETWORK; + } + auto const txID = ctx.tx.getTransactionID(); if (txID == beast::zero) diff --git a/src/ripple/core/Config.h b/src/ripple/core/Config.h index 2d440a1afd9..a395f5d54dd 100644 --- a/src/ripple/core/Config.h +++ b/src/ripple/core/Config.h @@ -138,6 +138,7 @@ class Config : public BasicConfig std::string START_LEDGER; // Network parameters + uint32_t NETWORK_ID = 0; // The number of fee units a reference transaction costs static constexpr FeeUnit32 TRANSACTION_FEE_BASE{10}; diff --git a/src/ripple/core/ConfigSections.h b/src/ripple/core/ConfigSections.h index ba0f209c0e9..4cff6c522e3 100644 --- a/src/ripple/core/ConfigSections.h +++ b/src/ripple/core/ConfigSections.h @@ -101,6 +101,7 @@ struct ConfigSection #define SECTION_LEDGER_REPLAY "ledger_replay" #define SECTION_BETA_RPC_API "beta_rpc_api" #define SECTION_SWEEP_INTERVAL "sweep_interval" +#define SECTION_NETWORK_ID "network_id" } // namespace ripple diff --git a/src/ripple/core/impl/Config.cpp b/src/ripple/core/impl/Config.cpp index c2cfb14d21d..9e8680056f5 100644 --- a/src/ripple/core/impl/Config.cpp +++ b/src/ripple/core/impl/Config.cpp @@ -479,6 +479,18 @@ Config::loadFromString(std::string const& fileContents) std::string strTemp; + if (getSingleSection(secConfig, SECTION_NETWORK_ID, strTemp, j_)) + { + if (strTemp == "main") + NETWORK_ID = 0; + else if (strTemp == "testnet") + NETWORK_ID = 1; + else if (strTemp == "devnet") + NETWORK_ID = 2; + else + NETWORK_ID = beast::lexicalCastThrow(strTemp); + } + if (getSingleSection(secConfig, SECTION_PEER_PRIVATE, strTemp, j_)) PEER_PRIVATE = beast::lexicalCastThrow(strTemp); diff --git a/src/ripple/protocol/SField.h b/src/ripple/protocol/SField.h index 253d956408f..f05a81babdf 100644 --- a/src/ripple/protocol/SField.h +++ b/src/ripple/protocol/SField.h @@ -354,6 +354,7 @@ extern SF_UINT16 const sfHookExecutionIndex; extern SF_UINT16 const sfHookApiVersion; // 32-bit integers (common) +extern SF_UINT32 const sfNetworkID; extern SF_UINT32 const sfFlags; extern SF_UINT32 const sfSourceTag; extern SF_UINT32 const sfSequence; diff --git a/src/ripple/protocol/TER.h b/src/ripple/protocol/TER.h index 38342f0c139..1ece0f2abcd 100644 --- a/src/ripple/protocol/TER.h +++ b/src/ripple/protocol/TER.h @@ -61,6 +61,9 @@ enum TELcodes : TERUnderlyingType { telCAN_NOT_QUEUE_BLOCKED, telCAN_NOT_QUEUE_FEE, telCAN_NOT_QUEUE_FULL, + telWRONG_NETWORK, + telREQUIRES_NETWORK_ID, + telNETWORK_ID_MAKES_TX_NON_CANONICAL }; //------------------------------------------------------------------------------ diff --git a/src/ripple/protocol/impl/SField.cpp b/src/ripple/protocol/impl/SField.cpp index 73098319b28..887dedd1ecd 100644 --- a/src/ripple/protocol/impl/SField.cpp +++ b/src/ripple/protocol/impl/SField.cpp @@ -104,6 +104,7 @@ CONSTRUCT_TYPED_SFIELD(sfHookExecutionIndex, "HookExecutionIndex", UINT16, CONSTRUCT_TYPED_SFIELD(sfHookApiVersion, "HookApiVersion", UINT16, 20); // 32-bit integers (common) +CONSTRUCT_TYPED_SFIELD(sfNetworkID, "NetworkID", UINT32, 1); CONSTRUCT_TYPED_SFIELD(sfFlags, "Flags", UINT32, 2); CONSTRUCT_TYPED_SFIELD(sfSourceTag, "SourceTag", UINT32, 3); CONSTRUCT_TYPED_SFIELD(sfSequence, "Sequence", UINT32, 4); diff --git a/src/ripple/protocol/impl/TER.cpp b/src/ripple/protocol/impl/TER.cpp index a845bdaeebc..2dfb45fd728 100644 --- a/src/ripple/protocol/impl/TER.cpp +++ b/src/ripple/protocol/impl/TER.cpp @@ -124,6 +124,9 @@ transResults() MAKE_ERROR(telCAN_NOT_QUEUE_BLOCKED, "Can not queue at this time: blocking transaction in queue."), MAKE_ERROR(telCAN_NOT_QUEUE_FEE, "Can not queue at this time: fee insufficient to replace queued transaction."), MAKE_ERROR(telCAN_NOT_QUEUE_FULL, "Can not queue at this time: queue is full."), + MAKE_ERROR(telWRONG_NETWORK, "Transaction specifies a Network ID that differs from that of the local node."), + MAKE_ERROR(telREQUIRES_NETWORK_ID, "Transactions submitted to this node/network must include a correct NetworkID field."), + MAKE_ERROR(telNETWORK_ID_MAKES_TX_NON_CANONICAL, "Transactions submitted to this node/network must NOT include a NetworkID field."), MAKE_ERROR(temMALFORMED, "Malformed transaction."), MAKE_ERROR(temBAD_AMOUNT, "Can only send positive amounts."), diff --git a/src/ripple/protocol/impl/TxFormats.cpp b/src/ripple/protocol/impl/TxFormats.cpp index ce0d5db921f..ea85beab648 100644 --- a/src/ripple/protocol/impl/TxFormats.cpp +++ b/src/ripple/protocol/impl/TxFormats.cpp @@ -40,6 +40,7 @@ TxFormats::TxFormats() {sfSigningPubKey, soeREQUIRED}, {sfTxnSignature, soeOPTIONAL}, {sfSigners, soeOPTIONAL}, // submit_multisigned + {sfNetworkID, soeOPTIONAL}, }; add(jss::AccountSet, diff --git a/src/ripple/protocol/jss.h b/src/ripple/protocol/jss.h index 1c5bf8463b0..050f5d3fcd3 100644 --- a/src/ripple/protocol/jss.h +++ b/src/ripple/protocol/jss.h @@ -71,6 +71,7 @@ JSS(Invalid); // JSS(LastLedgerSequence); // in: TransactionSign; field JSS(LedgerHashes); // ledger type. JSS(LimitAmount); // field. +JSS(NetworkID); // field. JSS(NFTokenBurn); // transaction type. JSS(NFTokenMint); // transaction type. JSS(NFTokenOffer); // ledger type. diff --git a/src/test/app/NetworkID_test.cpp b/src/test/app/NetworkID_test.cpp new file mode 100644 index 00000000000..693459239c1 --- /dev/null +++ b/src/test/app/NetworkID_test.cpp @@ -0,0 +1,153 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2020 Dev Null Productions + + 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 + +namespace ripple { +namespace test { + +class NetworkID_test : public beast::unit_test::suite +{ +public: + void + run() override + { + testNetworkID(); + } + + std::unique_ptr + makeNetworkConfig(uint32_t networkID) + { + using namespace jtx; + return envconfig([&](std::unique_ptr cfg) { + cfg->NETWORK_ID = networkID; + return cfg; + }); + } + + void + testNetworkID() + { + testcase( + "Require txn NetworkID to be specified (or not) depending on the " + "network ID of the node"); + using namespace jtx; + + auto const alice = Account{"alice"}; + + auto const runTx = [&](test::jtx::Env& env, + Json::Value const& jv, + TER expectedOutcome) { + env.memoize(env.master); + env.memoize(alice); + + // fund alice + { + Json::Value jv; + jv[jss::Account] = env.master.human(); + jv[jss::Destination] = alice.human(); + jv[jss::TransactionType] = "Payment"; + jv[jss::Amount] = "10000000000"; + if (env.app().config().NETWORK_ID > 1024) + jv[jss::NetworkID] = + std::to_string(env.app().config().NETWORK_ID); + + env(jv, fee(1000), sig(env.master)); + } + + // run tx + env(jv, fee(1000), ter(expectedOutcome)); + env.close(); + }; + + // test mainnet + { + test::jtx::Env env{*this, makeNetworkConfig(0)}; + BEAST_EXPECT(env.app().config().NETWORK_ID == 0); + + // try to submit a txn without network id, this should work + Json::Value jv; + jv[jss::Account] = alice.human(); + jv[jss::TransactionType] = jss::AccountSet; + runTx(env, jv, tesSUCCESS); + + // try to submit a txn with NetworkID present against a mainnet + // node, this will fail + jv[jss::NetworkID] = 0; + runTx(env, jv, telNETWORK_ID_MAKES_TX_NON_CANONICAL); + + // change network id to something else, should still return same + // error + jv[jss::NetworkID] = 10000; + runTx(env, jv, telNETWORK_ID_MAKES_TX_NON_CANONICAL); + } + + // any network up to and including networkid 1024 cannot support + // NetworkID + { + test::jtx::Env env{*this, makeNetworkConfig(1024)}; + BEAST_EXPECT(env.app().config().NETWORK_ID == 1024); + + // try to submit a txn without network id, this should work + Json::Value jv; + jv[jss::Account] = alice.human(); + jv[jss::TransactionType] = jss::AccountSet; + runTx(env, jv, tesSUCCESS); + + // now submit with a network id, this will fail + jv[jss::NetworkID] = 1024; + runTx(env, jv, telNETWORK_ID_MAKES_TX_NON_CANONICAL); + + jv[jss::NetworkID] = 1000; + runTx(env, jv, telNETWORK_ID_MAKES_TX_NON_CANONICAL); + } + + // any network above networkid 1024 will produce an error if fed a txn + // absent networkid + { + test::jtx::Env env{*this, makeNetworkConfig(1025)}; + BEAST_EXPECT(env.app().config().NETWORK_ID == 1025); + + // try to submit a txn without network id, this should not work + Json::Value jv; + jv[jss::Account] = alice.human(); + jv[jss::TransactionType] = jss::AccountSet; + runTx(env, jv, telREQUIRES_NETWORK_ID); + + // try to submit with wrong network id + jv[jss::NetworkID] = 0; + runTx(env, jv, telWRONG_NETWORK); + + jv[jss::NetworkID] = 1024; + runTx(env, jv, telWRONG_NETWORK); + + // submit the correct network id + jv[jss::NetworkID] = 1025; + runTx(env, jv, tesSUCCESS); + } + } +}; + +BEAST_DEFINE_TESTSUITE(NetworkID, app, ripple); + +} // namespace test +} // namespace ripple diff --git a/src/test/core/Config_test.cpp b/src/test/core/Config_test.cpp index da29fafaca2..efdc3db6ced 100644 --- a/src/test/core/Config_test.cpp +++ b/src/test/core/Config_test.cpp @@ -411,6 +411,71 @@ port_wss_admin } } + void + testNetworkID() + { + testcase("network id"); + std::string error; + Config c; + try + { + c.loadFromString(R"rippleConfig( +[network_id] +main +)rippleConfig"); + } + catch (std::runtime_error& e) + { + error = e.what(); + } + + BEAST_EXPECT(error == ""); + BEAST_EXPECT(c.NETWORK_ID == 0); + + try + { + c.loadFromString(R"rippleConfig( +)rippleConfig"); + } + catch (std::runtime_error& e) + { + error = e.what(); + } + + BEAST_EXPECT(error == ""); + BEAST_EXPECT(c.NETWORK_ID == 0); + + try + { + c.loadFromString(R"rippleConfig( +[network_id] +255 +)rippleConfig"); + } + catch (std::runtime_error& e) + { + error = e.what(); + } + + BEAST_EXPECT(error == ""); + BEAST_EXPECT(c.NETWORK_ID == 255); + + try + { + c.loadFromString(R"rippleConfig( +[network_id] +10000 +)rippleConfig"); + } + catch (std::runtime_error& e) + { + error = e.what(); + } + + BEAST_EXPECT(error == ""); + BEAST_EXPECT(c.NETWORK_ID == 10000); + } + void testValidatorsFile() { @@ -1151,6 +1216,7 @@ r.ripple.com 51235 testGetters(); testAmendment(); testOverlay(); + testNetworkID(); } }; diff --git a/src/test/jtx/impl/Env.cpp b/src/test/jtx/impl/Env.cpp index 900b9812dae..dbf723d3767 100644 --- a/src/test/jtx/impl/Env.cpp +++ b/src/test/jtx/impl/Env.cpp @@ -163,7 +163,10 @@ Env::lookup(AccountID const& id) const { auto const iter = map_.find(id); if (iter == map_.end()) + { + std::cout << "Unknown account: " << id << "\n"; Throw("Env::lookup:: unknown account ID"); + } return iter->second; } From 437ad3333435acd1673d6b17fb9b4e48a7972e1d Mon Sep 17 00:00:00 2001 From: RichardAH Date: Wed, 21 Dec 2022 12:54:03 +0100 Subject: [PATCH 2/4] Update src/ripple/app/tx/impl/Transactor.cpp Co-authored-by: Rome Reginelli --- src/ripple/app/tx/impl/Transactor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index 031f2d17d76..aaebf93d4c2 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -45,7 +45,7 @@ preflight0(PreflightContext const& ctx) if (nodeNID <= 1024) { - // legacy networks have ids less than 1024, these networks cannot + // legacy networks have IDs 1024 and below. These networks cannot // specify NetworkID in txn if (txNID) return telNETWORK_ID_MAKES_TX_NON_CANONICAL; From 368589c0ff0ba130a1a7f7586c7a096163b195b0 Mon Sep 17 00:00:00 2001 From: Richard Holland Date: Thu, 22 Dec 2022 15:42:43 +0000 Subject: [PATCH 3/4] [FOLD] add const, missing include --- src/ripple/app/tx/impl/Transactor.cpp | 4 ++-- src/test/app/NetworkID_test.cpp | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index aaebf93d4c2..45ed859cbb5 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -40,8 +40,8 @@ namespace ripple { NotTEC preflight0(PreflightContext const& ctx) { - uint32_t nodeNID = ctx.app.config().NETWORK_ID; - std::optional txNID = ctx.tx[~sfNetworkID]; + uint32_t const nodeNID = ctx.app.config().NETWORK_ID; + std::optional const txNID = ctx.tx[~sfNetworkID]; if (nodeNID <= 1024) { diff --git a/src/test/app/NetworkID_test.cpp b/src/test/app/NetworkID_test.cpp index 693459239c1..00f16cedf4b 100644 --- a/src/test/app/NetworkID_test.cpp +++ b/src/test/app/NetworkID_test.cpp @@ -21,6 +21,7 @@ #include #include #include +#include namespace ripple { namespace test { From a02b3bff877a7f1d01cc6943ecbb7a9d4a340471 Mon Sep 17 00:00:00 2001 From: Elliot Lee Date: Thu, 9 Feb 2023 21:54:41 -0800 Subject: [PATCH 4/4] [FOLD] fix: apply clang-format patch --- src/test/app/NetworkID_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/app/NetworkID_test.cpp b/src/test/app/NetworkID_test.cpp index 00f16cedf4b..e650667f842 100644 --- a/src/test/app/NetworkID_test.cpp +++ b/src/test/app/NetworkID_test.cpp @@ -19,9 +19,9 @@ #include #include +#include #include #include -#include namespace ripple { namespace test {