diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index de2d90d36ec..514f90fc5f5 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -6,3 +6,7 @@ e2384885f5f630c8f0ffe4bf21a169b433a16858 241b9ddde9e11beb7480600fd5ed90e1ef109b21 760f16f56835663d9286bd29294d074de26a7ba6 0eebe6a5f4246fced516d52b83ec4e7f47373edd +2189cc950c0cebb89e4e2fa3b2d8817205bf7cef +b9d007813378ad0ff45660dc07285b823c7e9855 +fe9a5365b8a52d4acc42eb27369247e6f238a4f9 +9a93577314e6a8d4b4a8368cc9d2b15a5d8303e8 diff --git a/.github/actions/build/action.yml b/.github/actions/build/action.yml index 79bbe9af075..6714369155f 100644 --- a/.github/actions/build/action.yml +++ b/.github/actions/build/action.yml @@ -20,6 +20,8 @@ runs: ${{ inputs.generator && format('-G "{0}"', inputs.generator) || '' }} \ -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \ -DCMAKE_BUILD_TYPE=${{ inputs.configuration }} \ + -Dtests=TRUE \ + -Dxrpld=TRUE \ ${{ inputs.cmake-args }} \ .. - name: build diff --git a/.github/actions/dependencies/action.yml b/.github/actions/dependencies/action.yml index d9687ff121e..50e2999018a 100644 --- a/.github/actions/dependencies/action.yml +++ b/.github/actions/dependencies/action.yml @@ -50,6 +50,8 @@ runs: conan install \ --output-folder . \ --build missing \ + --options tests=True \ + --options xrpld=True \ --settings build_type=${{ inputs.configuration }} \ .. - name: upload dependencies to remote diff --git a/.github/workflows/nix.yml b/.github/workflows/nix.yml index da61963b3f2..6b8261c5d69 100644 --- a/.github/workflows/nix.yml +++ b/.github/workflows/nix.yml @@ -222,7 +222,7 @@ jobs: - name: upload coverage report uses: wandalen/wretry.action@v1.4.10 with: - action: codecov/codecov-action@v4.3.0 + action: codecov/codecov-action@v4.5.0 with: | files: coverage.xml fail_ci_if_error: true @@ -232,3 +232,53 @@ jobs: token: ${{ secrets.CODECOV_TOKEN }} attempt_limit: 5 attempt_delay: 210000 # in milliseconds + + conan: + needs: dependencies + runs-on: [self-hosted, heavy] + container: rippleci/rippled-build-ubuntu:aaf5e3e + env: + build_dir: .build + configuration: Release + steps: + - name: download cache + uses: actions/download-artifact@v3 + with: + name: linux-gcc-${{ env.configuration }} + - name: extract cache + run: | + mkdir -p ~/.conan + tar -xzf conan.tar -C ~/.conan + - name: check environment + run: | + env | sort + echo ${PATH} | tr ':' '\n' + conan --version + cmake --version + - name: checkout + uses: actions/checkout@v4 + - name: dependencies + uses: ./.github/actions/dependencies + env: + CONAN_URL: http://18.143.149.228:8081/artifactory/api/conan/conan-non-prod + with: + configuration: ${{ env.configuration }} + - name: export + run: | + version=$(conan inspect --raw version .) + reference="xrpl/${version}@local/test" + conan remove -f ${reference} || true + conan export . local/test + echo "reference=${reference}" >> "${GITHUB_ENV}" + - name: build + run: | + cd examples/example + mkdir ${build_dir} + cd ${build_dir} + conan install .. --output-folder . \ + --require-override ${reference} --build missing + cmake .. \ + -DCMAKE_TOOLCHAIN_FILE:FILEPATH=./build/${configuration}/generators/conan_toolchain.cmake \ + -DCMAKE_BUILD_TYPE=${configuration} + cmake --build . + ./example | grep '^[[:digit:]]\+\.[[:digit:]]\+\.[[:digit:]]\+' diff --git a/BUILD.md b/BUILD.md index 0b9ef40e61b..b4201ef0437 100644 --- a/BUILD.md +++ b/BUILD.md @@ -98,6 +98,12 @@ Update the compiler settings: conan profile update settings.compiler.cppstd=20 default ``` +Configure Conan (1.x only) to use recipe revisions: + + ``` + conan config set general.revisions_enabled=1 + ``` + **Linux** developers will commonly have a default Conan [profile][] that compiles with GCC and links with libstdc++. If you are linking with libstdc++ (see profile setting `compiler.libcxx`), @@ -187,6 +193,17 @@ It patches their CMake to correctly import its dependencies. conan export --version 4.0.3 external/soci ``` +Export our [Conan recipe for NuDB](./external/nudb). +It fixes some source files to add missing `#include`s. + + + ``` + # Conan 1.x + conan export external/nudb nudb/2.0.8@ + # Conan 2.x + conan export --version 2.0.8 external/nudb + ``` + ### Build and Test 1. Create a build directory and move into it. @@ -242,7 +259,7 @@ It patches their CMake to correctly import its dependencies. Single-config generators: ``` - cmake -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake -DCMAKE_BUILD_TYPE=Release .. + cmake -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake -DCMAKE_BUILD_TYPE=Release -Dxrpld=ON -Dtests=ON .. ``` Pass the CMake variable [`CMAKE_BUILD_TYPE`][build_type] @@ -252,7 +269,7 @@ It patches their CMake to correctly import its dependencies. Multi-config generators: ``` - cmake -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake .. + cmake -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake -Dxrpld=ON -Dtests=ON .. ``` **Note:** You can pass build options for `rippled` in this step. @@ -343,7 +360,7 @@ Example use with some cmake variables set: ``` cd .build conan install .. --output-folder . --build missing --settings build_type=Debug -cmake -DCMAKE_BUILD_TYPE=Debug -Dcoverage=ON -Dcoverage_test_parallelism=2 -Dcoverage_format=html-details -Dcoverage_extra_args="--json coverage.json" -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake .. +cmake -DCMAKE_BUILD_TYPE=Debug -Dcoverage=ON -Dxrpld=ON -Dtests=ON -Dcoverage_test_parallelism=2 -Dcoverage_format=html-details -Dcoverage_extra_args="--json coverage.json" -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake .. cmake --build . --target coverage ``` diff --git a/Builds/levelization/levelization.sh b/Builds/levelization/levelization.sh index f8f21bb9d23..3c43a23092f 100755 --- a/Builds/levelization/levelization.sh +++ b/Builds/levelization/levelization.sh @@ -13,6 +13,9 @@ then git clean -ix fi +# Ensure all sorting is ASCII-order consistently across platforms. +export LANG=C + rm -rfv results mkdir results includes="$( pwd )/results/rawincludes.txt" diff --git a/cmake/RippledCore.cmake b/cmake/RippledCore.cmake index 9af3303f662..6a0060f7b32 100644 --- a/cmake/RippledCore.cmake +++ b/cmake/RippledCore.cmake @@ -91,56 +91,59 @@ target_link_libraries(xrpl.libxrpl xxHash::xxhash ) -add_executable(rippled) -if(unity) - set_target_properties(rippled PROPERTIES UNITY_BUILD ON) -endif() -if(tests) - target_compile_definitions(rippled PUBLIC ENABLE_TESTS) -endif() -target_include_directories(rippled - PRIVATE - $ -) - -file(GLOB_RECURSE sources CONFIGURE_DEPENDS - "${CMAKE_CURRENT_SOURCE_DIR}/src/xrpld/*.cpp" -) -target_sources(rippled PRIVATE ${sources}) +if(xrpld) + add_executable(rippled) + if(unity) + set_target_properties(rippled PROPERTIES UNITY_BUILD ON) + endif() + if(tests) + target_compile_definitions(rippled PUBLIC ENABLE_TESTS) + endif() + target_include_directories(rippled + PRIVATE + $ + ) -if(tests) file(GLOB_RECURSE sources CONFIGURE_DEPENDS - "${CMAKE_CURRENT_SOURCE_DIR}/src/test/*.cpp" + "${CMAKE_CURRENT_SOURCE_DIR}/src/xrpld/*.cpp" ) target_sources(rippled PRIVATE ${sources}) -endif() -target_link_libraries(rippled - Ripple::boost - Ripple::opts - Ripple::libs - xrpl.libxrpl -) -exclude_if_included(rippled) -# define a macro for tests that might need to -# be exluded or run differently in CI environment -if(is_ci) - target_compile_definitions(rippled PRIVATE RIPPLED_RUNNING_IN_CI) -endif () - -if(reporting) - set(suffix -reporting) - set_target_properties(rippled PROPERTIES OUTPUT_NAME rippled-reporting) - get_target_property(BIN_NAME rippled OUTPUT_NAME) - message(STATUS "Reporting mode build: rippled renamed ${BIN_NAME}") - target_compile_definitions(rippled PRIVATE RIPPLED_REPORTING) -endif() + if(tests) + file(GLOB_RECURSE sources CONFIGURE_DEPENDS + "${CMAKE_CURRENT_SOURCE_DIR}/src/test/*.cpp" + ) + target_sources(rippled PRIVATE ${sources}) + endif() -# any files that don't play well with unity should be added here -if(tests) - set_source_files_properties( - # these two seem to produce conflicts in beast teardown template methods - src/test/rpc/ValidatorRPC_test.cpp - src/test/rpc/ShardArchiveHandler_test.cpp - PROPERTIES SKIP_UNITY_BUILD_INCLUSION TRUE) + target_link_libraries(rippled + Ripple::boost + Ripple::opts + Ripple::libs + xrpl.libxrpl + ) + exclude_if_included(rippled) + # define a macro for tests that might need to + # be exluded or run differently in CI environment + if(is_ci) + target_compile_definitions(rippled PRIVATE RIPPLED_RUNNING_IN_CI) + endif () + + if(reporting) + set(suffix -reporting) + set_target_properties(rippled PROPERTIES OUTPUT_NAME rippled-reporting) + get_target_property(BIN_NAME rippled OUTPUT_NAME) + message(STATUS "Reporting mode build: rippled renamed ${BIN_NAME}") + target_compile_definitions(rippled PRIVATE RIPPLED_REPORTING) + endif() + + # any files that don't play well with unity should be added here + if(tests) + set_source_files_properties( + # these two seem to produce conflicts in beast teardown template methods + src/test/rpc/ValidatorRPC_test.cpp + src/test/rpc/ShardArchiveHandler_test.cpp + src/test/ledger/Invariants_test.cpp + PROPERTIES SKIP_UNITY_BUILD_INCLUSION TRUE) + endif() endif() diff --git a/cmake/RippledInstall.cmake b/cmake/RippledInstall.cmake index d92cecc24eb..3199c9a19b8 100644 --- a/cmake/RippledInstall.cmake +++ b/cmake/RippledInstall.cmake @@ -21,6 +21,13 @@ install( DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}" ) +if(NOT WIN32) + install( + CODE "file(CREATE_LINK xrpl \ + \${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_INCLUDEDIR}/ripple SYMBOLIC)" + ) +endif() + install (EXPORT RippleExports FILE RippleTargets.cmake NAMESPACE Ripple:: @@ -31,14 +38,9 @@ write_basic_package_version_file ( VERSION ${rippled_version} COMPATIBILITY SameMajorVersion) -if (is_root_project) +if (is_root_project AND TARGET rippled) install (TARGETS rippled RUNTIME DESTINATION bin) set_target_properties(rippled PROPERTIES INSTALL_RPATH_USE_LINK_PATH ON) - install ( - FILES - ${CMAKE_CURRENT_SOURCE_DIR}/cmake/RippleConfig.cmake - ${CMAKE_CURRENT_BINARY_DIR}/RippleConfigVersion.cmake - DESTINATION lib/cmake/ripple) # sample configs should not overwrite existing files # install if-not-exists workaround as suggested by # https://cmake.org/Bug/view.php?id=12646 @@ -53,18 +55,16 @@ if (is_root_project) copy_if_not_exists(\"${CMAKE_CURRENT_SOURCE_DIR}/cfg/rippled-example.cfg\" etc rippled.cfg) copy_if_not_exists(\"${CMAKE_CURRENT_SOURCE_DIR}/cfg/validators-example.txt\" etc validators.txt) ") + if(NOT WIN32) + install( + CODE "file(CREATE_LINK rippled${suffix} \ + \${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_BINDIR}/xrpld${suffix} SYMBOLIC)" + ) + endif() endif () -if(NOT WIN32) - install( - CODE "file(CREATE_LINK xrpl \ - \${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_INCLUDEDIR}/ripple SYMBOLIC)" - ) -endif() - -if(NOT WIN32) - install( - CODE "file(CREATE_LINK rippled${suffix} \ - \${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_BINDIR}/xrpld${suffix} SYMBOLIC)" - ) -endif() +install ( + FILES + ${CMAKE_CURRENT_SOURCE_DIR}/cmake/RippleConfig.cmake + ${CMAKE_CURRENT_BINARY_DIR}/RippleConfigVersion.cmake + DESTINATION lib/cmake/ripple) diff --git a/cmake/RippledSettings.cmake b/cmake/RippledSettings.cmake index fae09cc5d3f..a431bb61389 100644 --- a/cmake/RippledSettings.cmake +++ b/cmake/RippledSettings.cmake @@ -8,6 +8,8 @@ ProcessorCount(PROCESSOR_COUNT) option(assert "Enables asserts, even in release builds" OFF) +option(xrpld "Build xrpld" ON) + option(reporting "Build rippled with reporting mode enabled" OFF) option(tests "Build tests" ON) diff --git a/conanfile.py b/conanfile.py index 68b0a7cd405..425fee8b682 100644 --- a/conanfile.py +++ b/conanfile.py @@ -21,6 +21,7 @@ class Xrpl(ConanFile): 'static': [True, False], 'tests': [True, False], 'unity': [True, False], + 'xrpld': [True, False], } requires = [ @@ -47,8 +48,9 @@ class Xrpl(ConanFile): 'rocksdb': True, 'shared': False, 'static': True, - 'tests': True, + 'tests': False, 'unity': False, + 'xrpld': False, 'cassandra-cpp-driver/*:shared': False, 'cassandra-cpp-driver/*:use_atomic': None, @@ -142,6 +144,7 @@ def generate(self): tc.variables['BUILD_SHARED_LIBS'] = self.options.shared tc.variables['static'] = self.options.static tc.variables['unity'] = self.options.unity + tc.variables['xrpld'] = self.options.xrpld tc.generate() def build(self): diff --git a/examples/example/CMakeLists.txt b/examples/example/CMakeLists.txt new file mode 100644 index 00000000000..83aa24880d1 --- /dev/null +++ b/examples/example/CMakeLists.txt @@ -0,0 +1,16 @@ +cmake_minimum_required(VERSION 3.21) + +set(name example) +set(version 0.1.0) + +project( + ${name} + VERSION ${version} + LANGUAGES CXX +) + +find_package(xrpl REQUIRED) + +add_executable(example) +target_sources(example PRIVATE src/example.cpp) +target_link_libraries(example PRIVATE xrpl::libxrpl) diff --git a/examples/example/conanfile.py b/examples/example/conanfile.py new file mode 100644 index 00000000000..be3750bf9e9 --- /dev/null +++ b/examples/example/conanfile.py @@ -0,0 +1,59 @@ +from conan import ConanFile, conan_version +from conan.tools.cmake import CMake, cmake_layout + +class Example(ConanFile): + + def set_name(self): + if self.name is None: + self.name = 'example' + + def set_version(self): + if self.version is None: + self.version = '0.1.0' + + license = 'ISC' + author = 'John Freeman ' + + settings = 'os', 'compiler', 'build_type', 'arch' + options = {'shared': [True, False], 'fPIC': [True, False]} + default_options = { + 'shared': False, + 'fPIC': True, + 'xrpl:xrpld': False, + } + + requires = ['xrpl/2.2.0-rc1@jfreeman/nodestore'] + generators = ['CMakeDeps', 'CMakeToolchain'] + + exports_sources = [ + 'CMakeLists.txt', + 'cmake/*', + 'external/*', + 'include/*', + 'src/*', + ] + + # For out-of-source build. + # https://docs.conan.io/en/latest/reference/build_helpers/cmake.html#configure + no_copy_source = True + + def layout(self): + cmake_layout(self) + + def config_options(self): + if self.settings.os == 'Windows': + del self.options.fPIC + + def build(self): + cmake = CMake(self) + cmake.configure(variables={'BUILD_TESTING': 'NO'}) + cmake.build() + + def package(self): + cmake = CMake(self) + cmake.install() + + def package_info(self): + path = f'{self.package_folder}/share/{self.name}/cpp_info.py' + with open(path, 'r') as file: + exec(file.read(), {}, {'self': self.cpp_info}) diff --git a/examples/example/src/example.cpp b/examples/example/src/example.cpp new file mode 100644 index 00000000000..7ff07f6ea4d --- /dev/null +++ b/examples/example/src/example.cpp @@ -0,0 +1,8 @@ +#include + +#include + +int main(int argc, char const** argv) { + std::printf("%s\n", ripple::BuildInfo::getVersionString().c_str()); + return 0; +} diff --git a/external/nudb/conandata.yml b/external/nudb/conandata.yml new file mode 100644 index 00000000000..721129f88e7 --- /dev/null +++ b/external/nudb/conandata.yml @@ -0,0 +1,10 @@ +sources: + "2.0.8": + url: "https://github.com/CPPAlliance/NuDB/archive/2.0.8.tar.gz" + sha256: "9b71903d8ba111cd893ab064b9a8b6ac4124ed8bd6b4f67250205bc43c7f13a8" +patches: + "2.0.8": + - patch_file: "patches/2.0.8-0001-add-include-stdexcept-for-msvc.patch" + patch_description: "Fix build for MSVC by including stdexcept" + patch_type: "portability" + patch_source: "https://github.com/cppalliance/NuDB/pull/100/files" diff --git a/external/nudb/conanfile.py b/external/nudb/conanfile.py new file mode 100644 index 00000000000..a046e2ba898 --- /dev/null +++ b/external/nudb/conanfile.py @@ -0,0 +1,72 @@ +import os + +from conan import ConanFile +from conan.tools.build import check_min_cppstd +from conan.tools.files import apply_conandata_patches, copy, export_conandata_patches, get +from conan.tools.layout import basic_layout + +required_conan_version = ">=1.52.0" + + +class NudbConan(ConanFile): + name = "nudb" + description = "A fast key/value insert-only database for SSD drives in C++11" + license = "BSL-1.0" + url = "https://github.com/conan-io/conan-center-index" + homepage = "https://github.com/CPPAlliance/NuDB" + topics = ("header-only", "KVS", "insert-only") + + package_type = "header-library" + settings = "os", "arch", "compiler", "build_type" + no_copy_source = True + + @property + def _min_cppstd(self): + return 11 + + def export_sources(self): + export_conandata_patches(self) + + def layout(self): + basic_layout(self, src_folder="src") + + def requirements(self): + self.requires("boost/1.83.0") + + def package_id(self): + self.info.clear() + + def validate(self): + if self.settings.compiler.cppstd: + check_min_cppstd(self, self._min_cppstd) + + def source(self): + get(self, **self.conan_data["sources"][self.version], strip_root=True) + + def build(self): + apply_conandata_patches(self) + + def package(self): + copy(self, "LICENSE*", + dst=os.path.join(self.package_folder, "licenses"), + src=self.source_folder) + copy(self, "*", + dst=os.path.join(self.package_folder, "include"), + src=os.path.join(self.source_folder, "include")) + + def package_info(self): + self.cpp_info.bindirs = [] + self.cpp_info.libdirs = [] + + self.cpp_info.set_property("cmake_target_name", "NuDB") + self.cpp_info.set_property("cmake_target_aliases", ["NuDB::nudb"]) + self.cpp_info.set_property("cmake_find_mode", "both") + + self.cpp_info.components["core"].set_property("cmake_target_name", "nudb") + self.cpp_info.components["core"].names["cmake_find_package"] = "nudb" + self.cpp_info.components["core"].names["cmake_find_package_multi"] = "nudb" + self.cpp_info.components["core"].requires = ["boost::thread", "boost::system"] + + # TODO: to remove in conan v2 once cmake_find_package_* generators removed + self.cpp_info.names["cmake_find_package"] = "NuDB" + self.cpp_info.names["cmake_find_package_multi"] = "NuDB" diff --git a/external/nudb/patches/2.0.8-0001-add-include-stdexcept-for-msvc.patch b/external/nudb/patches/2.0.8-0001-add-include-stdexcept-for-msvc.patch new file mode 100644 index 00000000000..2d5264f3ce4 --- /dev/null +++ b/external/nudb/patches/2.0.8-0001-add-include-stdexcept-for-msvc.patch @@ -0,0 +1,24 @@ +diff --git a/include/nudb/detail/stream.hpp b/include/nudb/detail/stream.hpp +index 6c07bf1..e0ce8ed 100644 +--- a/include/nudb/detail/stream.hpp ++++ b/include/nudb/detail/stream.hpp +@@ -14,6 +14,7 @@ + #include + #include + #include ++#include + + namespace nudb { + namespace detail { +diff --git a/include/nudb/impl/context.ipp b/include/nudb/impl/context.ipp +index beb7058..ffde0b3 100644 +--- a/include/nudb/impl/context.ipp ++++ b/include/nudb/impl/context.ipp +@@ -9,6 +9,7 @@ + #define NUDB_IMPL_CONTEXT_IPP + + #include ++#include + + namespace nudb { + diff --git a/external/secp256k1/src/secp256k1.c b/external/secp256k1/src/secp256k1.c index bdbd97cc408..a95992c5dd2 100644 --- a/external/secp256k1/src/secp256k1.c +++ b/external/secp256k1/src/secp256k1.c @@ -526,7 +526,7 @@ static int secp256k1_ecdsa_sign_inner(const secp256k1_context* ctx, secp256k1_sc break; } is_nonce_valid = secp256k1_scalar_set_b32_seckey(&non, nonce32); - /* The nonce is still secret here, but it being invalid is is less likely than 1:2^255. */ + /* The nonce is still secret here, but it being invalid is less likely than 1:2^255. */ secp256k1_declassify(ctx, &is_nonce_valid, sizeof(is_nonce_valid)); if (is_nonce_valid) { ret = secp256k1_ecdsa_sig_sign(&ctx->ecmult_gen_ctx, r, s, &sec, &msg, &non, recid); diff --git a/include/xrpl/protocol/Feature.h b/include/xrpl/protocol/Feature.h index 013f4c58adf..a00d6b85c1b 100644 --- a/include/xrpl/protocol/Feature.h +++ b/include/xrpl/protocol/Feature.h @@ -80,7 +80,7 @@ namespace detail { // Feature.cpp. Because it's only used to reserve storage, and determine how // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than // the actual number of amendments. A LogicError on startup will verify this. -static constexpr std::size_t numFeatures = 77; +static constexpr std::size_t numFeatures = 79; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated @@ -369,6 +369,8 @@ extern uint256 const fixAMMv1_1; extern uint256 const featureNFTokenMintOffer; extern uint256 const fixReducedOffersV2; extern uint256 const fixEnforceNFTokenTrustline; +extern uint256 const fixInnerObjTemplate2; +extern uint256 const featureInvariantsV1_1; extern uint256 const fixNFTokenPageLinks; } // namespace ripple diff --git a/include/xrpl/protocol/Indexes.h b/include/xrpl/protocol/Indexes.h index d57525121de..f179bbacfab 100644 --- a/include/xrpl/protocol/Indexes.h +++ b/include/xrpl/protocol/Indexes.h @@ -29,6 +29,7 @@ #include #include #include +#include #include namespace ripple { @@ -306,6 +307,26 @@ getTicketIndex(AccountID const& account, std::uint32_t uSequence); uint256 getTicketIndex(AccountID const& account, SeqProxy ticketSeq); +template +struct keyletDesc +{ + std::function function; + Json::StaticString expectedLEName; + bool includeInTests; +}; + +// This list should include all of the keylet functions that take a single +// AccountID parameter. +std::array, 6> const directAccountKeylets{ + {{&keylet::account, jss::AccountRoot, false}, + {&keylet::ownerDir, jss::DirectoryNode, true}, + {&keylet::signers, jss::SignerList, true}, + // It's normally impossible to create an item at nftpage_min, but + // test it anyway, since the invariant checks for it. + {&keylet::nftpage_min, jss::NFTokenPage, true}, + {&keylet::nftpage_max, jss::NFTokenPage, true}, + {&keylet::did, jss::DID, true}}}; + } // namespace ripple #endif diff --git a/include/xrpl/protocol/STObject.h b/include/xrpl/protocol/STObject.h index 5259994a976..b3cef83de5f 100644 --- a/include/xrpl/protocol/STObject.h +++ b/include/xrpl/protocol/STObject.h @@ -44,7 +44,6 @@ namespace ripple { class STArray; -class Rules; inline void throwFieldNotFound(SField const& field) @@ -105,7 +104,7 @@ class STObject : public STBase, public CountedObject explicit STObject(SField const& name); static STObject - makeInnerObject(SField const& name, Rules const& rules); + makeInnerObject(SField const& name); iterator begin() const; diff --git a/include/xrpl/resource/Fees.h b/include/xrpl/resource/Fees.h index d3750ec8282..1eb1a9bd725 100644 --- a/include/xrpl/resource/Fees.h +++ b/include/xrpl/resource/Fees.h @@ -25,43 +25,38 @@ namespace ripple { namespace Resource { +// clang-format off /** Schedule of fees charged for imposing load on the server. */ /** @{ */ -extern Charge const - feeInvalidRequest; // A request that we can immediately tell is invalid +extern Charge const feeInvalidRequest; // A request that we can immediately + // tell is invalid extern Charge const feeRequestNoReply; // A request that we cannot satisfy -extern Charge const feeInvalidSignature; // An object whose signature we had to - // check and it failed +extern Charge const feeInvalidSignature; // An object whose signature we had + // to check and it failed extern Charge const feeUnwantedData; // Data we have no use for -extern Charge const feeBadData; // Data we have to verify before rejecting +extern Charge const feeBadData; // Data we have to verify before + // rejecting // RPC loads -extern Charge const - feeInvalidRPC; // An RPC request that we can immediately tell is invalid. -extern Charge const feeReferenceRPC; // A default "reference" unspecified load -extern Charge const feeExceptionRPC; // An RPC load that causes an exception -extern Charge const feeLightRPC; // A normal RPC command -extern Charge const feeLowBurdenRPC; // A slightly burdensome RPC load -extern Charge const feeMediumBurdenRPC; // A somewhat burdensome RPC load -extern Charge const feeHighBurdenRPC; // A very burdensome RPC load -extern Charge const feePathFindUpdate; // An update to an existing PF request +extern Charge const feeInvalidRPC; // An RPC request that we can + // immediately tell is invalid. +extern Charge const feeReferenceRPC; // A default "reference" unspecified + // load +extern Charge const feeExceptionRPC; // RPC load that causes an exception +extern Charge const feeMediumBurdenRPC; // A somewhat burdensome RPC load +extern Charge const feeHighBurdenRPC; // A very burdensome RPC load // Peer loads extern Charge const feeLightPeer; // Requires no reply -extern Charge const feeLowBurdenPeer; // Quick/cheap, slight reply extern Charge const feeMediumBurdenPeer; // Requires some work extern Charge const feeHighBurdenPeer; // Extensive work -// Good things -extern Charge const - feeNewTrustedNote; // A new transaction/validation/proposal we trust -extern Charge const feeNewValidTx; // A new, valid transaction -extern Charge const feeSatisfiedRequest; // Data we requested - // Administrative -extern Charge const feeWarning; // The cost of receiving a warning -extern Charge const feeDrop; // The cost of being dropped for excess load +extern Charge const feeWarning; // The cost of receiving a warning +extern Charge const feeDrop; // The cost of being dropped for + // excess load /** @} */ +// clang-format on } // namespace Resource } // namespace ripple diff --git a/src/libxrpl/protocol/BuildInfo.cpp b/src/libxrpl/protocol/BuildInfo.cpp index 9139f29e83c..b0a7bcc9ed7 100644 --- a/src/libxrpl/protocol/BuildInfo.cpp +++ b/src/libxrpl/protocol/BuildInfo.cpp @@ -33,7 +33,7 @@ namespace BuildInfo { // and follow the format described at http://semver.org/ //------------------------------------------------------------------------------ // clang-format off -char const* const versionString = "2.2.0" +char const* const versionString = "2.3.0-b1" // clang-format on #if defined(DEBUG) || defined(SANITIZER) diff --git a/src/libxrpl/protocol/Feature.cpp b/src/libxrpl/protocol/Feature.cpp index d42288d2259..078369bf20c 100644 --- a/src/libxrpl/protocol/Feature.cpp +++ b/src/libxrpl/protocol/Feature.cpp @@ -496,7 +496,11 @@ REGISTER_FIX (fixAMMv1_1, Supported::yes, VoteBehavior::De REGISTER_FEATURE(NFTokenMintOffer, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FIX (fixReducedOffersV2, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FIX (fixEnforceNFTokenTrustline, Supported::yes, VoteBehavior::DefaultNo); +REGISTER_FIX (fixInnerObjTemplate2, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FIX (fixNFTokenPageLinks, Supported::yes, VoteBehavior::DefaultNo); +// InvariantsV1_1 will be changes to Supported::yes when all the +// invariants expected to be included under it are complete. +REGISTER_FEATURE(InvariantsV1_1, Supported::no, VoteBehavior::DefaultNo); // The following amendments are obsolete, but must remain supported // because they could potentially get enabled. diff --git a/src/libxrpl/protocol/STObject.cpp b/src/libxrpl/protocol/STObject.cpp index a369974c2d6..bde83ec31a1 100644 --- a/src/libxrpl/protocol/STObject.cpp +++ b/src/libxrpl/protocol/STObject.cpp @@ -61,10 +61,19 @@ STObject::STObject(SerialIter& sit, SField const& name, int depth) noexcept( } STObject -STObject::makeInnerObject(SField const& name, Rules const& rules) +STObject::makeInnerObject(SField const& name) { STObject obj{name}; - if (rules.enabled(fixInnerObjTemplate)) + + // The if is complicated because inner object templates were added in + // two phases: + // 1. If there are no available Rules, then always apply the template. + // 2. fixInnerObjTemplate added templates to two AMM inner objects. + // 3. fixInnerObjTemplate2 added templates to all remaining inner objects. + std::optional const& rules = getCurrentTransactionRules(); + bool const isAMMObj = name == sfAuctionSlot || name == sfVoteEntry; + if (!rules || (rules->enabled(fixInnerObjTemplate) && isAMMObj) || + (rules->enabled(fixInnerObjTemplate2) && !isAMMObj)) { if (SOTemplate const* elements = InnerObjectFormats::getInstance().findSOTemplateBySField(name)) diff --git a/src/libxrpl/protocol/XChainAttestations.cpp b/src/libxrpl/protocol/XChainAttestations.cpp index f87f1c3e681..82e73445693 100644 --- a/src/libxrpl/protocol/XChainAttestations.cpp +++ b/src/libxrpl/protocol/XChainAttestations.cpp @@ -203,7 +203,8 @@ AttestationClaim::AttestationClaim(Json::Value const& v) STObject AttestationClaim::toSTObject() const { - STObject o{sfXChainClaimAttestationCollectionElement}; + STObject o = + STObject::makeInnerObject(sfXChainClaimAttestationCollectionElement); addHelper(o); o[sfXChainClaimID] = claimID; if (dst) @@ -345,7 +346,8 @@ AttestationCreateAccount::AttestationCreateAccount( STObject AttestationCreateAccount::toSTObject() const { - STObject o{sfXChainCreateAccountAttestationCollectionElement}; + STObject o = STObject::makeInnerObject( + sfXChainCreateAccountAttestationCollectionElement); addHelper(o); o[sfXChainAccountCreateCount] = createCount; @@ -497,7 +499,7 @@ XChainClaimAttestation::XChainClaimAttestation( STObject XChainClaimAttestation::toSTObject() const { - STObject o{sfXChainClaimProofSig}; + STObject o = STObject::makeInnerObject(sfXChainClaimProofSig); o[sfAttestationSignerAccount] = STAccount{sfAttestationSignerAccount, keyAccount}; o[sfPublicKey] = publicKey; @@ -609,7 +611,7 @@ XChainCreateAccountAttestation::XChainCreateAccountAttestation( STObject XChainCreateAccountAttestation::toSTObject() const { - STObject o{sfXChainCreateAccountProofSig}; + STObject o = STObject::makeInnerObject(sfXChainCreateAccountProofSig); o[sfAttestationSignerAccount] = STAccount{sfAttestationSignerAccount, keyAccount}; diff --git a/src/test/app/Escrow_test.cpp b/src/test/app/Escrow_test.cpp index 813f26da736..0f465a14f4d 100644 --- a/src/test/app/Escrow_test.cpp +++ b/src/test/app/Escrow_test.cpp @@ -19,7 +19,7 @@ #include #include -#include +#include #include #include #include diff --git a/src/test/app/NFTokenBurn_test.cpp b/src/test/app/NFTokenBurn_test.cpp index b25d9900972..a84ac63da9d 100644 --- a/src/test/app/NFTokenBurn_test.cpp +++ b/src/test/app/NFTokenBurn_test.cpp @@ -18,7 +18,6 @@ //============================================================================== #include -#include #include #include #include diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index a8eae4a25ec..0dffe5e388f 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -18,7 +18,7 @@ //============================================================================== #include -#include +#include #include #include #include diff --git a/src/test/csf/CollectorRef.h b/src/test/csf/CollectorRef.h index 0f8d90c8ec9..72d1e9545d8 100644 --- a/src/test/csf/CollectorRef.h +++ b/src/test/csf/CollectorRef.h @@ -40,7 +40,7 @@ namespace csf { level when adding to the simulation. The example code below demonstrates the reason for storing the collector - as a reference. The collector's lifetime will generally be be longer than + as a reference. The collector's lifetime will generally be longer than the simulation; perhaps several simulations are run for a single collector instance. The collector potentially stores lots of data as well, so the simulation needs to point to the single instance, rather than requiring diff --git a/src/test/csf/Digraph.h b/src/test/csf/Digraph.h index 2c6b356bf02..3f079eac17c 100644 --- a/src/test/csf/Digraph.h +++ b/src/test/csf/Digraph.h @@ -220,7 +220,7 @@ class Digraph @param fileName The output file (creates) @param vertexName A invokable T vertexName(Vertex const &) that returns the name target use for the vertex in the file - T must be be ostream-able + T must be ostream-able */ template void diff --git a/src/test/ledger/Directory_test.cpp b/src/test/ledger/Directory_test.cpp index 1a14feff061..4904b6e6fbf 100644 --- a/src/test/ledger/Directory_test.cpp +++ b/src/test/ledger/Directory_test.cpp @@ -17,7 +17,6 @@ #include #include -#include #include #include #include diff --git a/src/test/ledger/Invariants_test.cpp b/src/test/ledger/Invariants_test.cpp index 8d856ed52b8..8d7b08fa1ab 100644 --- a/src/test/ledger/Invariants_test.cpp +++ b/src/test/ledger/Invariants_test.cpp @@ -18,6 +18,7 @@ //============================================================================== #include +#include #include #include #include @@ -26,10 +27,21 @@ #include #include +#include + namespace ripple { class Invariants_test : public beast::unit_test::suite { + // The optional Preclose function is used to process additional transactions + // on the ledger after creating two accounts, but before closing it, and + // before the Precheck function. These should only be valid functions, and + // not direct manipulations. Preclose is not commonly used. + using Preclose = std::function; + // 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. @@ -38,22 +50,42 @@ class Invariants_test : public beast::unit_test::suite test::jtx::Account const& b, ApplyContext& ac)>; + /** Run a specific test case to put the ledger into a state that will be + * detected by an invariant. Simulates the actions of a transaction that + * would violate an invariant. + * + * @param expect_logs One or more messages related to the failing invariant + * that should be in the log output + * @precheck See "Precheck" above + * @fee If provided, the fee amount paid by the simulated transaction. + * @tx A mock transaction that took the actions to trigger the invariant. In + * most cases, only the type matters. + * @ters The TER results expected on the two passes of the invariant + * checker. + * @preclose See "Preclose" above. Note that @preclose runs *before* + * @precheck, but is the last parameter for historical reasons + * + */ void doInvariantCheck( std::vector const& expect_logs, Precheck const& precheck, XRPAmount fee = XRPAmount{}, STTx tx = STTx{ttACCOUNT_SET, [](STObject&) {}}, - std::initializer_list ters = { - tecINVARIANT_FAILED, - tefINVARIANT_FAILED}) + std::initializer_list ters = + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}, + Preclose const& preclose = {}) { using namespace test::jtx; - Env env{*this}; + FeatureBitset amendments = + supported_amendments() | featureInvariantsV1_1; + Env env{*this, amendments}; - Account A1{"A1"}; - Account A2{"A2"}; + Account const A1{"A1"}; + Account const A2{"A2"}; env.fund(XRP(1000), A1, A2); + if (preclose) + BEAST_EXPECT(preclose(A1, A2, env)); env.close(); OpenView ov{*env.current()}; @@ -161,6 +193,165 @@ class Invariants_test : public beast::unit_test::suite STTx{ttACCOUNT_DELETE, [](STObject& tx) {}}); } + void + testAccountRootsDeletedClean() + { + using namespace test::jtx; + testcase << "account root deletion left artifact"; + + for (auto const& keyletInfo : directAccountKeylets) + { + // TODO: Use structured binding once LLVM 16 is the minimum + // supported version. See also: + // https://github.com/llvm/llvm-project/issues/48582 + // https://github.com/llvm/llvm-project/commit/127bf44385424891eb04cff8e52d3f157fc2cb7c + if (!keyletInfo.includeInTests) + continue; + auto const& keyletfunc = keyletInfo.function; + auto const& type = keyletInfo.expectedLEName; + + using namespace std::string_literals; + + doInvariantCheck( + {{"account deletion left behind a "s + type.c_str() + + " object"}}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + // Add an object to the ledger for account A1, then delete + // A1 + auto const a1 = A1.id(); + auto const sleA1 = ac.view().peek(keylet::account(a1)); + if (!sleA1) + return false; + + auto const key = std::invoke(keyletfunc, a1); + auto const newSLE = std::make_shared(key); + ac.view().insert(newSLE); + ac.view().erase(sleA1); + + return true; + }, + XRPAmount{}, + STTx{ttACCOUNT_DELETE, [](STObject& tx) {}}); + }; + + // NFT special case + doInvariantCheck( + {{"account deletion left behind a NFTokenPage object"}}, + [&](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; + }, + XRPAmount{}, + STTx{ttACCOUNT_DELETE, [](STObject& tx) {}}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}, + [&](Account const& A1, Account const&, Env& env) { + // Preclose callback to mint the NFT which will be deleted in + // the Precheck callback above. + env(token::mint(A1)); + + return true; + }); + + // AMM special cases + AccountID ammAcctID; + uint256 ammKey; + Issue ammIssue; + doInvariantCheck( + {{"account deletion left behind a DirectoryNode object"}}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + // Delete the AMM account without cleaning up the directory or + // deleting the AMM object + auto const sle = ac.view().peek(keylet::account(ammAcctID)); + if (!sle) + return false; + + BEAST_EXPECT(sle->at(~sfAMMID)); + BEAST_EXPECT(sle->at(~sfAMMID) == ammKey); + + ac.view().erase(sle); + + return true; + }, + XRPAmount{}, + STTx{ttAMM_WITHDRAW, [](STObject& tx) {}}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + // Preclose callback to create the AMM which will be partially + // deleted in the Precheck callback above. + AMM const amm(env, A1, XRP(100), A1["USD"](50)); + ammAcctID = amm.ammAccount(); + ammKey = amm.ammID(); + ammIssue = amm.lptIssue(); + return true; + }); + doInvariantCheck( + {{"account deletion left behind a AMM object"}}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + // Delete all the AMM's trust lines, remove the AMM from the AMM + // account's directory (this deletes the directory), and delete + // the AMM account. Do not delete the AMM object. + auto const sle = ac.view().peek(keylet::account(ammAcctID)); + if (!sle) + return false; + + BEAST_EXPECT(sle->at(~sfAMMID)); + BEAST_EXPECT(sle->at(~sfAMMID) == ammKey); + + for (auto const& trustKeylet : + {keylet::line(ammAcctID, A1["USD"]), + keylet::line(A1, ammIssue)}) + { + if (auto const line = ac.view().peek(trustKeylet); !line) + { + return false; + } + else + { + STAmount const lowLimit = line->at(sfLowLimit); + STAmount const highLimit = line->at(sfHighLimit); + BEAST_EXPECT( + trustDelete( + ac.view(), + line, + lowLimit.getIssuer(), + highLimit.getIssuer(), + ac.journal) == tesSUCCESS); + } + } + + auto const ammSle = ac.view().peek(keylet::amm(ammKey)); + if (!BEAST_EXPECT(ammSle)) + return false; + auto const ownerDirKeylet = keylet::ownerDir(ammAcctID); + + BEAST_EXPECT(ac.view().dirRemove( + ownerDirKeylet, ammSle->at(sfOwnerNode), ammKey, false)); + BEAST_EXPECT( + !ac.view().exists(ownerDirKeylet) || + ac.view().emptyDirDelete(ownerDirKeylet)); + + ac.view().erase(sle); + + return true; + }, + XRPAmount{}, + STTx{ttAMM_WITHDRAW, [](STObject& tx) {}}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + // Preclose callback to create the AMM which will be partially + // deleted in the Precheck callback above. + AMM const amm(env, A1, XRP(100), A1["USD"](50)); + ammAcctID = amm.ammAccount(); + ammKey = amm.ammID(); + ammIssue = amm.lptIssue(); + return true; + }); + } + void testTypesMatch() { @@ -174,7 +365,7 @@ class Invariants_test : public beast::unit_test::suite auto const sle = ac.view().peek(keylet::account(A1.id())); if (!sle) return false; - auto sleNew = std::make_shared(ltTICKET, sle->key()); + auto const sleNew = std::make_shared(ltTICKET, sle->key()); ac.rawView().rawReplace(sleNew); return true; }); @@ -190,7 +381,7 @@ class Invariants_test : public beast::unit_test::suite // 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( + auto const sleNew = std::make_shared( keylet::escrow(A1, (*sle)[sfSequence] + 2)); // We don't use ltNICKNAME directly since it's marked deprecated @@ -230,7 +421,7 @@ class Invariants_test : public beast::unit_test::suite auto const sle = ac.view().peek(keylet::account(A1.id())); if (!sle) return false; - STAmount nonNative(A2["USD"](51)); + STAmount const nonNative(A2["USD"](51)); sle->setFieldAmount(sfBalance, nonNative); ac.view().update(sle); return true; @@ -419,7 +610,7 @@ class Invariants_test : public beast::unit_test::suite [](Account const&, Account const&, ApplyContext& ac) { // Insert a new account root created by a non-payment into // the view. - const Account A3{"A3"}; + Account const A3{"A3"}; Keylet const acctKeylet = keylet::account(A3); auto const sleNew = std::make_shared(acctKeylet); ac.view().insert(sleNew); @@ -431,13 +622,13 @@ class Invariants_test : public beast::unit_test::suite [](Account const&, Account const&, ApplyContext& ac) { // Insert two new account roots into the view. { - const Account A3{"A3"}; + Account const A3{"A3"}; Keylet const acctKeylet = keylet::account(A3); auto const sleA3 = std::make_shared(acctKeylet); ac.view().insert(sleA3); } { - const Account A4{"A4"}; + Account const A4{"A4"}; Keylet const acctKeylet = keylet::account(A4); auto const sleA4 = std::make_shared(acctKeylet); ac.view().insert(sleA4); @@ -449,7 +640,7 @@ class Invariants_test : public beast::unit_test::suite {{"account created with wrong starting sequence number"}}, [](Account const&, Account const&, ApplyContext& ac) { // Insert a new account root with the wrong starting sequence. - const Account A3{"A3"}; + Account const A3{"A3"}; Keylet const acctKeylet = keylet::account(A3); auto const sleNew = std::make_shared(acctKeylet); sleNew->setFieldU32(sfSequence, ac.view().seq() + 1); @@ -613,6 +804,7 @@ class Invariants_test : public beast::unit_test::suite { testXRPNotCreated(); testAccountRootsNotRemoved(); + testAccountRootsDeletedClean(); testTypesMatch(); testNoXRPTrustLine(); testXRPBalanceCheck(); diff --git a/src/test/rpc/AccountObjects_test.cpp b/src/test/rpc/AccountObjects_test.cpp index 18291bf2b95..0757ca40bf0 100644 --- a/src/test/rpc/AccountObjects_test.cpp +++ b/src/test/rpc/AccountObjects_test.cpp @@ -20,10 +20,12 @@ #include #include #include +#include #include #include #include #include +#include #include @@ -556,11 +558,11 @@ class AccountObjects_test : public beast::unit_test::suite Env env(*this, features); // Make a lambda we can use to get "account_objects" easily. - auto acct_objs = [&env]( - AccountID const& acct, - std::optional const& type, - std::optional limit = std::nullopt, - std::optional marker = std::nullopt) { + auto acctObjs = [&env]( + AccountID const& acct, + std::optional const& type, + std::optional limit = std::nullopt, + std::optional marker = std::nullopt) { Json::Value params; params[jss::account] = to_string(acct); if (type) @@ -574,32 +576,42 @@ class AccountObjects_test : public beast::unit_test::suite }; // Make a lambda that easily identifies the size of account objects. - auto acct_objs_is_size = [](Json::Value const& resp, unsigned size) { + auto acctObjsIsSize = [](Json::Value const& resp, unsigned size) { return resp[jss::result][jss::account_objects].isArray() && (resp[jss::result][jss::account_objects].size() == size); }; + // Make a lambda that checks if the response has error for invalid type + auto acctObjsTypeIsInvalid = [](Json::Value const& resp) { + return resp[jss::result].isMember(jss::error) && + resp[jss::result][jss::error_message] == + "Invalid field \'type\'."; + }; + env.fund(XRP(10000), gw, alice); env.close(); // Since the account is empty now, all account objects should come // back empty. - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::account), 0)); - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::amendments), 0)); - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::check), 0)); - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::deposit_preauth), 0)); - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::directory), 0)); - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::escrow), 0)); - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::fee), 0)); - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::hashes), 0)); - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::nft_page), 0)); - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::offer), 0)); - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::payment_channel), 0)); - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::signer_list), 0)); - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::state), 0)); - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::ticket), 0)); - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::amm), 0)); - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::did), 0)); + BEAST_EXPECT(acctObjsIsSize(acctObjs(gw, jss::account), 0)); + BEAST_EXPECT(acctObjsIsSize(acctObjs(gw, jss::check), 0)); + BEAST_EXPECT(acctObjsIsSize(acctObjs(gw, jss::deposit_preauth), 0)); + BEAST_EXPECT(acctObjsIsSize(acctObjs(gw, jss::escrow), 0)); + BEAST_EXPECT(acctObjsIsSize(acctObjs(gw, jss::nft_page), 0)); + BEAST_EXPECT(acctObjsIsSize(acctObjs(gw, jss::offer), 0)); + BEAST_EXPECT(acctObjsIsSize(acctObjs(gw, jss::payment_channel), 0)); + BEAST_EXPECT(acctObjsIsSize(acctObjs(gw, jss::signer_list), 0)); + BEAST_EXPECT(acctObjsIsSize(acctObjs(gw, jss::state), 0)); + BEAST_EXPECT(acctObjsIsSize(acctObjs(gw, jss::ticket), 0)); + BEAST_EXPECT(acctObjsIsSize(acctObjs(gw, jss::amm), 0)); + BEAST_EXPECT(acctObjsIsSize(acctObjs(gw, jss::did), 0)); + + // we expect invalid field type reported for the following types + BEAST_EXPECT(acctObjsTypeIsInvalid(acctObjs(gw, jss::amendments))); + BEAST_EXPECT(acctObjsTypeIsInvalid(acctObjs(gw, jss::directory))); + BEAST_EXPECT(acctObjsTypeIsInvalid(acctObjs(gw, jss::fee))); + BEAST_EXPECT(acctObjsTypeIsInvalid(acctObjs(gw, jss::hashes))); + BEAST_EXPECT(acctObjsTypeIsInvalid(acctObjs(gw, jss::NegativeUNL))); // gw mints an NFT so we can find it. uint256 const nftID{token::getNextID(env, gw, 0u, tfTransferable)}; @@ -607,8 +619,8 @@ class AccountObjects_test : public beast::unit_test::suite env.close(); { // Find the NFToken page and make sure it's the right one. - Json::Value const resp = acct_objs(gw, jss::nft_page); - BEAST_EXPECT(acct_objs_is_size(resp, 1)); + Json::Value const resp = acctObjs(gw, jss::nft_page); + BEAST_EXPECT(acctObjsIsSize(resp, 1)); auto const& nftPage = resp[jss::result][jss::account_objects][0u]; BEAST_EXPECT(nftPage[sfNFTokens.jsonName].size() == 1); @@ -624,8 +636,8 @@ class AccountObjects_test : public beast::unit_test::suite env.close(); { // Find the trustline and make sure it's the right one. - Json::Value const resp = acct_objs(gw, jss::state); - BEAST_EXPECT(acct_objs_is_size(resp, 1)); + Json::Value const resp = acctObjs(gw, jss::state); + BEAST_EXPECT(acctObjsIsSize(resp, 1)); auto const& state = resp[jss::result][jss::account_objects][0u]; BEAST_EXPECT(state[sfBalance.jsonName][jss::value].asInt() == -5); @@ -637,8 +649,8 @@ class AccountObjects_test : public beast::unit_test::suite env.close(); { // Find the check. - Json::Value const resp = acct_objs(gw, jss::check); - BEAST_EXPECT(acct_objs_is_size(resp, 1)); + Json::Value const resp = acctObjs(gw, jss::check); + BEAST_EXPECT(acctObjsIsSize(resp, 1)); auto const& check = resp[jss::result][jss::account_objects][0u]; BEAST_EXPECT(check[sfAccount.jsonName] == gw.human()); @@ -650,8 +662,8 @@ class AccountObjects_test : public beast::unit_test::suite env.close(); { // Find the preauthorization. - Json::Value const resp = acct_objs(gw, jss::deposit_preauth); - BEAST_EXPECT(acct_objs_is_size(resp, 1)); + Json::Value const resp = acctObjs(gw, jss::deposit_preauth); + BEAST_EXPECT(acctObjsIsSize(resp, 1)); auto const& preauth = resp[jss::result][jss::account_objects][0u]; BEAST_EXPECT(preauth[sfAccount.jsonName] == gw.human()); @@ -672,8 +684,8 @@ class AccountObjects_test : public beast::unit_test::suite } { // Find the escrow. - Json::Value const resp = acct_objs(gw, jss::escrow); - BEAST_EXPECT(acct_objs_is_size(resp, 1)); + Json::Value const resp = acctObjs(gw, jss::escrow); + BEAST_EXPECT(acctObjsIsSize(resp, 1)); auto const& escrow = resp[jss::result][jss::account_objects][0u]; BEAST_EXPECT(escrow[sfAccount.jsonName] == gw.human()); @@ -686,7 +698,7 @@ class AccountObjects_test : public beast::unit_test::suite Env scEnv(*this, envconfig(port_increment, 3), features); x.createScBridgeObjects(scEnv); - auto scenv_acct_objs = [&](Account const& acct, char const* type) { + auto scEnvAcctObjs = [&](Account const& acct, char const* type) { Json::Value params; params[jss::account] = acct.human(); params[jss::type] = type; @@ -695,9 +707,9 @@ class AccountObjects_test : public beast::unit_test::suite }; Json::Value const resp = - scenv_acct_objs(Account::master, jss::bridge); + scEnvAcctObjs(Account::master, jss::bridge); - BEAST_EXPECT(acct_objs_is_size(resp, 1)); + BEAST_EXPECT(acctObjsIsSize(resp, 1)); auto const& acct_bridge = resp[jss::result][jss::account_objects][0u]; BEAST_EXPECT( @@ -733,7 +745,7 @@ class AccountObjects_test : public beast::unit_test::suite scEnv(xchain_create_claim_id(x.scBob, x.jvb, x.reward, x.mcBob)); scEnv.close(); - auto scenv_acct_objs = [&](Account const& acct, char const* type) { + auto scEnvAcctObjs = [&](Account const& acct, char const* type) { Json::Value params; params[jss::account] = acct.human(); params[jss::type] = type; @@ -744,8 +756,8 @@ class AccountObjects_test : public beast::unit_test::suite { // Find the xchain sequence number for Andrea. Json::Value const resp = - scenv_acct_objs(x.scAlice, jss::xchain_owned_claim_id); - BEAST_EXPECT(acct_objs_is_size(resp, 1)); + scEnvAcctObjs(x.scAlice, jss::xchain_owned_claim_id); + BEAST_EXPECT(acctObjsIsSize(resp, 1)); auto const& xchain_seq = resp[jss::result][jss::account_objects][0u]; @@ -757,8 +769,8 @@ class AccountObjects_test : public beast::unit_test::suite { // and the one for Bob Json::Value const resp = - scenv_acct_objs(x.scBob, jss::xchain_owned_claim_id); - BEAST_EXPECT(acct_objs_is_size(resp, 1)); + scEnvAcctObjs(x.scBob, jss::xchain_owned_claim_id); + BEAST_EXPECT(acctObjsIsSize(resp, 1)); auto const& xchain_seq = resp[jss::result][jss::account_objects][0u]; @@ -790,7 +802,7 @@ class AccountObjects_test : public beast::unit_test::suite x.signers[0])); scEnv.close(); - auto scenv_acct_objs = [&](Account const& acct, char const* type) { + auto scEnvAcctObjs = [&](Account const& acct, char const* type) { Json::Value params; params[jss::account] = acct.human(); params[jss::type] = type; @@ -800,9 +812,9 @@ class AccountObjects_test : public beast::unit_test::suite { // Find the xchain_create_account_claim_id - Json::Value const resp = scenv_acct_objs( + Json::Value const resp = scEnvAcctObjs( Account::master, jss::xchain_owned_create_account_claim_id); - BEAST_EXPECT(acct_objs_is_size(resp, 1)); + BEAST_EXPECT(acctObjsIsSize(resp, 1)); auto const& xchain_create_account_claim_id = resp[jss::result][jss::account_objects][0u]; @@ -821,8 +833,8 @@ class AccountObjects_test : public beast::unit_test::suite env.close(); { // Find the offer. - Json::Value const resp = acct_objs(gw, jss::offer); - BEAST_EXPECT(acct_objs_is_size(resp, 1)); + Json::Value const resp = acctObjs(gw, jss::offer); + BEAST_EXPECT(acctObjsIsSize(resp, 1)); auto const& offer = resp[jss::result][jss::account_objects][0u]; BEAST_EXPECT(offer[sfAccount.jsonName] == gw.human()); @@ -846,8 +858,8 @@ class AccountObjects_test : public beast::unit_test::suite } { // Find the payment channel. - Json::Value const resp = acct_objs(gw, jss::payment_channel); - BEAST_EXPECT(acct_objs_is_size(resp, 1)); + Json::Value const resp = acctObjs(gw, jss::payment_channel); + BEAST_EXPECT(acctObjsIsSize(resp, 1)); auto const& payChan = resp[jss::result][jss::account_objects][0u]; BEAST_EXPECT(payChan[sfAccount.jsonName] == gw.human()); @@ -868,8 +880,8 @@ class AccountObjects_test : public beast::unit_test::suite } { // Find the DID. - Json::Value const resp = acct_objs(gw, jss::did); - BEAST_EXPECT(acct_objs_is_size(resp, 1)); + Json::Value const resp = acctObjs(gw, jss::did); + BEAST_EXPECT(acctObjsIsSize(resp, 1)); auto const& did = resp[jss::result][jss::account_objects][0u]; BEAST_EXPECT(did[sfAccount.jsonName] == gw.human()); @@ -880,8 +892,8 @@ class AccountObjects_test : public beast::unit_test::suite env.close(); { // Find the signer list. - Json::Value const resp = acct_objs(gw, jss::signer_list); - BEAST_EXPECT(acct_objs_is_size(resp, 1)); + Json::Value const resp = acctObjs(gw, jss::signer_list); + BEAST_EXPECT(acctObjsIsSize(resp, 1)); auto const& signerList = resp[jss::result][jss::account_objects][0u]; @@ -896,8 +908,8 @@ class AccountObjects_test : public beast::unit_test::suite env.close(); { // Find the ticket. - Json::Value const resp = acct_objs(gw, jss::ticket); - BEAST_EXPECT(acct_objs_is_size(resp, 1)); + Json::Value const resp = acctObjs(gw, jss::ticket); + BEAST_EXPECT(acctObjsIsSize(resp, 1)); auto const& ticket = resp[jss::result][jss::account_objects][0u]; BEAST_EXPECT(ticket[sfAccount.jsonName] == gw.human()); @@ -925,7 +937,7 @@ class AccountObjects_test : public beast::unit_test::suite std::uint32_t const expectedAccountObjects{ static_cast(std::size(expectedLedgerTypes))}; - if (BEAST_EXPECT(acct_objs_is_size(resp, expectedAccountObjects))) + if (BEAST_EXPECT(acctObjsIsSize(resp, expectedAccountObjects))) { auto const& aobjs = resp[jss::result][jss::account_objects]; std::vector gotLedgerTypes; @@ -948,7 +960,7 @@ class AccountObjects_test : public beast::unit_test::suite params[jss::type] = jss::escrow; auto resp = env.rpc("json", "account_objects", to_string(params)); - if (BEAST_EXPECT(acct_objs_is_size(resp, 1u))) + if (BEAST_EXPECT(acctObjsIsSize(resp, 1u))) { auto const& aobjs = resp[jss::result][jss::account_objects]; BEAST_EXPECT(aobjs[0u]["LedgerEntryType"] == jss::Escrow); @@ -969,7 +981,7 @@ class AccountObjects_test : public beast::unit_test::suite auto expectObjects = [&](Json::Value const& resp, std::vector const& types) -> bool { - if (!acct_objs_is_size(resp, types.size())) + if (!acctObjsIsSize(resp, types.size())) return false; std::vector typesOut; getTypes(resp, typesOut); @@ -983,13 +995,13 @@ class AccountObjects_test : public beast::unit_test::suite BEAST_EXPECT(lines[jss::lines].size() == 3); // request AMM only, doesn't depend on the limit BEAST_EXPECT( - acct_objs_is_size(acct_objs(amm.ammAccount(), jss::amm), 1)); + acctObjsIsSize(acctObjs(amm.ammAccount(), jss::amm), 1)); // request first two objects - auto resp = acct_objs(amm.ammAccount(), std::nullopt, 2); + auto resp = acctObjs(amm.ammAccount(), std::nullopt, 2); std::vector typesOut; getTypes(resp, typesOut); // request next two objects - resp = acct_objs( + resp = acctObjs( amm.ammAccount(), std::nullopt, 10, @@ -1003,7 +1015,7 @@ class AccountObjects_test : public beast::unit_test::suite jss::RippleState.c_str(), jss::RippleState.c_str()})); // filter by state: there are three trustlines - resp = acct_objs(amm.ammAccount(), jss::state, 10); + resp = acctObjs(amm.ammAccount(), jss::state, 10); BEAST_EXPECT(expectObjects( resp, {jss::RippleState.c_str(), @@ -1011,11 +1023,18 @@ class AccountObjects_test : public beast::unit_test::suite jss::RippleState.c_str()})); // AMM account doesn't own offers BEAST_EXPECT( - acct_objs_is_size(acct_objs(amm.ammAccount(), jss::offer), 0)); + acctObjsIsSize(acctObjs(amm.ammAccount(), jss::offer), 0)); // gw account doesn't own AMM object - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::amm), 0)); + BEAST_EXPECT(acctObjsIsSize(acctObjs(gw, jss::amm), 0)); } + // we still expect invalid field type reported for the following types + BEAST_EXPECT(acctObjsTypeIsInvalid(acctObjs(gw, jss::amendments))); + BEAST_EXPECT(acctObjsTypeIsInvalid(acctObjs(gw, jss::directory))); + BEAST_EXPECT(acctObjsTypeIsInvalid(acctObjs(gw, jss::fee))); + BEAST_EXPECT(acctObjsTypeIsInvalid(acctObjs(gw, jss::hashes))); + BEAST_EXPECT(acctObjsTypeIsInvalid(acctObjs(gw, jss::NegativeUNL))); + // Run up the number of directory entries so gw has two // directory nodes. for (int d = 1'000'032; d >= 1'000'000; --d) @@ -1025,11 +1044,129 @@ class AccountObjects_test : public beast::unit_test::suite } // Verify that the non-returning types still don't return anything. - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::account), 0)); - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::amendments), 0)); - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::directory), 0)); - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::fee), 0)); - BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::hashes), 0)); + BEAST_EXPECT(acctObjsIsSize(acctObjs(gw, jss::account), 0)); + } + + void + testNFTsMarker() + { + // there's some bug found in account_nfts method that it did not + // return invalid params when providing unassociated nft marker. + // this test tests both situations when providing valid nft marker + // and unassociated nft marker. + testcase("NFTsMarker"); + + using namespace jtx; + Env env(*this); + + Account const bob{"bob"}; + env.fund(XRP(10000), bob); + + static constexpr unsigned nftsSize = 10; + for (unsigned i = 0; i < nftsSize; i++) + { + env(token::mint(bob, 0)); + } + + env.close(); + + // save the NFTokenIDs to use later + std::vector tokenIDs; + { + Json::Value params; + params[jss::account] = bob.human(); + params[jss::ledger_index] = "validated"; + Json::Value const resp = + env.rpc("json", "account_nfts", to_string(params)); + Json::Value const& nfts = resp[jss::result][jss::account_nfts]; + for (Json::Value const& nft : nfts) + tokenIDs.push_back(nft["NFTokenID"]); + } + + // this lambda function is used to check if the account_nfts method + // returns the correct token information. lastIndex is used to query the + // last marker. + auto compareNFTs = [&tokenIDs, &env, &bob]( + unsigned const limit, unsigned const lastIndex) { + Json::Value params; + params[jss::account] = bob.human(); + params[jss::limit] = limit; + params[jss::marker] = tokenIDs[lastIndex]; + params[jss::ledger_index] = "validated"; + Json::Value const resp = + env.rpc("json", "account_nfts", to_string(params)); + + if (resp[jss::result].isMember(jss::error)) + return false; + + Json::Value const& nfts = resp[jss::result][jss::account_nfts]; + unsigned const nftsCount = tokenIDs.size() - lastIndex - 1 < limit + ? tokenIDs.size() - lastIndex - 1 + : limit; + + if (nfts.size() != nftsCount) + return false; + + for (unsigned i = 0; i < nftsCount; i++) + { + if (nfts[i]["NFTokenID"] != tokenIDs[lastIndex + 1 + i]) + return false; + } + + return true; + }; + + // test a valid marker which is equal to the third tokenID + BEAST_EXPECT(compareNFTs(4, 2)); + + // test a valid marker which is equal to the 8th tokenID + BEAST_EXPECT(compareNFTs(4, 7)); + + // lambda that holds common code for invalid cases. + auto testInvalidMarker = [&env, &bob]( + auto marker, char const* errorMessage) { + Json::Value params; + params[jss::account] = bob.human(); + params[jss::limit] = 4; + params[jss::ledger_index] = jss::validated; + params[jss::marker] = marker; + Json::Value const resp = + env.rpc("json", "account_nfts", to_string(params)); + return resp[jss::result][jss::error_message] == errorMessage; + }; + + // test an invalid marker that is not a string + BEAST_EXPECT( + testInvalidMarker(17, "Invalid field \'marker\', not string.")); + + // test an invalid marker that has a non-hex character + BEAST_EXPECT(testInvalidMarker( + "00000000F51DFC2A09D62CBBA1DFBDD4691DAC96AD98B900000000000000000G", + "Invalid field \'marker\'.")); + + // this lambda function is used to create some fake marker using given + // taxon and sequence because we want to test some unassociated markers + // later + auto createFakeNFTMarker = [](AccountID const& issuer, + std::uint32_t taxon, + std::uint32_t tokenSeq, + std::uint16_t flags = 0, + std::uint16_t fee = 0) { + // the marker has the exact same format as an NFTokenID + return to_string(NFTokenMint::createNFTokenID( + flags, fee, issuer, nft::toTaxon(taxon), tokenSeq)); + }; + + // test an unassociated marker which does not exist in the NFTokenIDs + BEAST_EXPECT(testInvalidMarker( + createFakeNFTMarker(bob.id(), 0x000000000, 0x00000000), + "Invalid field \'marker\'.")); + + // test an unassociated marker which exceeds the maximum value of the + // existing NFTokenID + BEAST_EXPECT(testInvalidMarker( + createFakeNFTMarker(bob.id(), 0xFFFFFFFF, 0xFFFFFFFF), + "Invalid field \'marker\'.")); } void @@ -1039,6 +1176,7 @@ class AccountObjects_test : public beast::unit_test::suite testUnsteppedThenStepped(); testUnsteppedThenSteppedWithNFTs(); testObjectTypes(); + testNFTsMarker(); } }; diff --git a/src/test/rpc/Feature_test.cpp b/src/test/rpc/Feature_test.cpp index 488255542f2..12d4b27745c 100644 --- a/src/test/rpc/Feature_test.cpp +++ b/src/test/rpc/Feature_test.cpp @@ -229,9 +229,28 @@ class Feature_test : public beast::unit_test::suite using namespace test::jtx; Env env{*this}; - auto jrr = env.rpc("feature", "AllTheThings")[jss::result]; - BEAST_EXPECT(jrr[jss::error] == "badFeature"); - BEAST_EXPECT(jrr[jss::error_message] == "Feature unknown or invalid."); + auto testInvalidParam = [&](auto const& param) { + Json::Value params; + params[jss::feature] = param; + auto jrr = + env.rpc("json", "feature", to_string(params))[jss::result]; + BEAST_EXPECT(jrr[jss::error] == "invalidParams"); + BEAST_EXPECT(jrr[jss::error_message] == "Invalid parameters."); + }; + + testInvalidParam(1); + testInvalidParam(1.1); + testInvalidParam(true); + testInvalidParam(Json::Value(Json::nullValue)); + testInvalidParam(Json::Value(Json::objectValue)); + testInvalidParam(Json::Value(Json::arrayValue)); + + { + auto jrr = env.rpc("feature", "AllTheThings")[jss::result]; + BEAST_EXPECT(jrr[jss::error] == "badFeature"); + BEAST_EXPECT( + jrr[jss::error_message] == "Feature unknown or invalid."); + } } void diff --git a/src/test/rpc/LedgerData_test.cpp b/src/test/rpc/LedgerData_test.cpp index 34dfb3e011b..1e4f97a935f 100644 --- a/src/test/rpc/LedgerData_test.cpp +++ b/src/test/rpc/LedgerData_test.cpp @@ -300,201 +300,214 @@ class LedgerData_test : public beast::unit_test::suite { // Put a bunch of different LedgerEntryTypes into a ledger using namespace test::jtx; - using namespace std::chrono; - Env env{*this, envconfig(validator, "")}; - Account const gw{"gateway"}; - auto const USD = gw["USD"]; - env.fund(XRP(100000), gw); - - auto makeRequest = [&env](Json::StaticString const& type) { - Json::Value jvParams; - jvParams[jss::ledger_index] = "current"; - jvParams[jss::type] = type; - return env.rpc( - "json", - "ledger_data", - boost::lexical_cast(jvParams))[jss::result]; - }; - - // Assert that state is an empty array. - for (auto const& type : - {jss::amendments, - jss::check, - jss::directory, - jss::offer, - jss::signer_list, - jss::state, - jss::ticket, - jss::escrow, - jss::payment_channel, - jss::deposit_preauth}) + // Make sure fixInnerObjTemplate2 doesn't break amendments. + for (FeatureBitset const& features : + {supported_amendments() - fixInnerObjTemplate2, + supported_amendments() | fixInnerObjTemplate2}) { - auto const jrr = makeRequest(type); - BEAST_EXPECT(checkArraySize(jrr[jss::state], 0)); - } + using namespace std::chrono; + Env env{*this, envconfig(validator, ""), features}; + + Account const gw{"gateway"}; + auto const USD = gw["USD"]; + env.fund(XRP(100000), gw); + + auto makeRequest = [&env](Json::StaticString const& type) { + Json::Value jvParams; + jvParams[jss::ledger_index] = "current"; + jvParams[jss::type] = type; + return env.rpc( + "json", + "ledger_data", + boost::lexical_cast(jvParams))[jss::result]; + }; + + // Assert that state is an empty array. + for (auto const& type : + {jss::amendments, + jss::check, + jss::directory, + jss::offer, + jss::signer_list, + jss::state, + jss::ticket, + jss::escrow, + jss::payment_channel, + jss::deposit_preauth}) + { + auto const jrr = makeRequest(type); + BEAST_EXPECT(checkArraySize(jrr[jss::state], 0)); + } - int const num_accounts = 10; + int const num_accounts = 10; - for (auto i = 0; i < num_accounts; i++) - { - Account const bob{std::string("bob") + std::to_string(i)}; - env.fund(XRP(1000), bob); - } - env(offer(Account{"bob0"}, USD(100), XRP(100))); - env.trust(Account{"bob2"}["USD"](100), Account{"bob3"}); + for (auto i = 0; i < num_accounts; i++) + { + Account const bob{std::string("bob") + std::to_string(i)}; + env.fund(XRP(1000), bob); + } + env(offer(Account{"bob0"}, USD(100), XRP(100))); + env.trust(Account{"bob2"}["USD"](100), Account{"bob3"}); - auto majorities = getMajorityAmendments(*env.closed()); - for (int i = 0; i <= 256; ++i) - { - env.close(); - majorities = getMajorityAmendments(*env.closed()); - if (!majorities.empty()) - break; - } - env(signers( - Account{"bob0"}, 1, {{Account{"bob1"}, 1}, {Account{"bob2"}, 1}})); - env(ticket::create(env.master, 1)); + auto majorities = getMajorityAmendments(*env.closed()); + for (int i = 0; i <= 256; ++i) + { + env.close(); + majorities = getMajorityAmendments(*env.closed()); + if (!majorities.empty()) + break; + } - { - Json::Value jv; - jv[jss::TransactionType] = jss::EscrowCreate; - jv[jss::Flags] = tfUniversal; - jv[jss::Account] = Account{"bob5"}.human(); - jv[jss::Destination] = Account{"bob6"}.human(); - jv[jss::Amount] = XRP(50).value().getJson(JsonOptions::none); - jv[sfFinishAfter.fieldName] = NetClock::time_point{env.now() + 10s} - .time_since_epoch() - .count(); - env(jv); - } + env(signers( + Account{"bob0"}, + 1, + {{Account{"bob1"}, 1}, {Account{"bob2"}, 1}})); + env(ticket::create(env.master, 1)); - { - Json::Value jv; - jv[jss::TransactionType] = jss::PaymentChannelCreate; - jv[jss::Flags] = tfUniversal; - jv[jss::Account] = Account{"bob6"}.human(); - jv[jss::Destination] = Account{"bob7"}.human(); - jv[jss::Amount] = XRP(100).value().getJson(JsonOptions::none); - jv[jss::SettleDelay] = NetClock::duration{10s}.count(); - jv[sfPublicKey.fieldName] = strHex(Account{"bob6"}.pk().slice()); - jv[sfCancelAfter.fieldName] = NetClock::time_point{env.now() + 300s} - .time_since_epoch() - .count(); - env(jv); - } + { + Json::Value jv; + jv[jss::TransactionType] = jss::EscrowCreate; + jv[jss::Flags] = tfUniversal; + jv[jss::Account] = Account{"bob5"}.human(); + jv[jss::Destination] = Account{"bob6"}.human(); + jv[jss::Amount] = XRP(50).value().getJson(JsonOptions::none); + jv[sfFinishAfter.fieldName] = + NetClock::time_point{env.now() + 10s} + .time_since_epoch() + .count(); + env(jv); + } - env(check::create("bob6", "bob7", XRP(100))); + { + Json::Value jv; + jv[jss::TransactionType] = jss::PaymentChannelCreate; + jv[jss::Flags] = tfUniversal; + jv[jss::Account] = Account{"bob6"}.human(); + jv[jss::Destination] = Account{"bob7"}.human(); + jv[jss::Amount] = XRP(100).value().getJson(JsonOptions::none); + jv[jss::SettleDelay] = NetClock::duration{10s}.count(); + jv[sfPublicKey.fieldName] = + strHex(Account{"bob6"}.pk().slice()); + jv[sfCancelAfter.fieldName] = + NetClock::time_point{env.now() + 300s} + .time_since_epoch() + .count(); + env(jv); + } - // bob9 DepositPreauths bob4 and bob8. - env(deposit::auth(Account{"bob9"}, Account{"bob4"})); - env(deposit::auth(Account{"bob9"}, Account{"bob8"})); - env.close(); + env(check::create("bob6", "bob7", XRP(100))); - // Now fetch each type + // bob9 DepositPreauths bob4 and bob8. + env(deposit::auth(Account{"bob9"}, Account{"bob4"})); + env(deposit::auth(Account{"bob9"}, Account{"bob8"})); + env.close(); - { // jvParams[jss::type] = "account"; - auto const jrr = makeRequest(jss::account); - BEAST_EXPECT(checkArraySize(jrr[jss::state], 12)); - for (auto const& j : jrr[jss::state]) - BEAST_EXPECT(j["LedgerEntryType"] == jss::AccountRoot); - } + // Now fetch each type - { // jvParams[jss::type] = "amendments"; - auto const jrr = makeRequest(jss::amendments); - BEAST_EXPECT(checkArraySize(jrr[jss::state], 1)); - for (auto const& j : jrr[jss::state]) - BEAST_EXPECT(j["LedgerEntryType"] == jss::Amendments); - } + { // jvParams[jss::type] = "account"; + auto const jrr = makeRequest(jss::account); + BEAST_EXPECT(checkArraySize(jrr[jss::state], 12)); + for (auto const& j : jrr[jss::state]) + BEAST_EXPECT(j["LedgerEntryType"] == jss::AccountRoot); + } - { // jvParams[jss::type] = "check"; - auto const jrr = makeRequest(jss::check); - BEAST_EXPECT(checkArraySize(jrr[jss::state], 1)); - for (auto const& j : jrr[jss::state]) - BEAST_EXPECT(j["LedgerEntryType"] == jss::Check); - } + { // jvParams[jss::type] = "amendments"; + auto const jrr = makeRequest(jss::amendments); + BEAST_EXPECT(checkArraySize(jrr[jss::state], 1)); + for (auto const& j : jrr[jss::state]) + BEAST_EXPECT(j["LedgerEntryType"] == jss::Amendments); + } - { // jvParams[jss::type] = "directory"; - auto const jrr = makeRequest(jss::directory); - BEAST_EXPECT(checkArraySize(jrr[jss::state], 9)); - for (auto const& j : jrr[jss::state]) - BEAST_EXPECT(j["LedgerEntryType"] == jss::DirectoryNode); - } + { // jvParams[jss::type] = "check"; + auto const jrr = makeRequest(jss::check); + BEAST_EXPECT(checkArraySize(jrr[jss::state], 1)); + for (auto const& j : jrr[jss::state]) + BEAST_EXPECT(j["LedgerEntryType"] == jss::Check); + } - { // jvParams[jss::type] = "fee"; - auto const jrr = makeRequest(jss::fee); - BEAST_EXPECT(checkArraySize(jrr[jss::state], 1)); - for (auto const& j : jrr[jss::state]) - BEAST_EXPECT(j["LedgerEntryType"] == jss::FeeSettings); - } + { // jvParams[jss::type] = "directory"; + auto const jrr = makeRequest(jss::directory); + BEAST_EXPECT(checkArraySize(jrr[jss::state], 9)); + for (auto const& j : jrr[jss::state]) + BEAST_EXPECT(j["LedgerEntryType"] == jss::DirectoryNode); + } - { // jvParams[jss::type] = "hashes"; - auto const jrr = makeRequest(jss::hashes); - BEAST_EXPECT(checkArraySize(jrr[jss::state], 2)); - for (auto const& j : jrr[jss::state]) - BEAST_EXPECT(j["LedgerEntryType"] == jss::LedgerHashes); - } + { // jvParams[jss::type] = "fee"; + auto const jrr = makeRequest(jss::fee); + BEAST_EXPECT(checkArraySize(jrr[jss::state], 1)); + for (auto const& j : jrr[jss::state]) + BEAST_EXPECT(j["LedgerEntryType"] == jss::FeeSettings); + } - { // jvParams[jss::type] = "offer"; - auto const jrr = makeRequest(jss::offer); - BEAST_EXPECT(checkArraySize(jrr[jss::state], 1)); - for (auto const& j : jrr[jss::state]) - BEAST_EXPECT(j["LedgerEntryType"] == jss::Offer); - } + { // jvParams[jss::type] = "hashes"; + auto const jrr = makeRequest(jss::hashes); + BEAST_EXPECT(checkArraySize(jrr[jss::state], 2)); + for (auto const& j : jrr[jss::state]) + BEAST_EXPECT(j["LedgerEntryType"] == jss::LedgerHashes); + } - { // jvParams[jss::type] = "signer_list"; - auto const jrr = makeRequest(jss::signer_list); - BEAST_EXPECT(checkArraySize(jrr[jss::state], 1)); - for (auto const& j : jrr[jss::state]) - BEAST_EXPECT(j["LedgerEntryType"] == jss::SignerList); - } + { // jvParams[jss::type] = "offer"; + auto const jrr = makeRequest(jss::offer); + BEAST_EXPECT(checkArraySize(jrr[jss::state], 1)); + for (auto const& j : jrr[jss::state]) + BEAST_EXPECT(j["LedgerEntryType"] == jss::Offer); + } - { // jvParams[jss::type] = "state"; - auto const jrr = makeRequest(jss::state); - BEAST_EXPECT(checkArraySize(jrr[jss::state], 1)); - for (auto const& j : jrr[jss::state]) - BEAST_EXPECT(j["LedgerEntryType"] == jss::RippleState); - } + { // jvParams[jss::type] = "signer_list"; + auto const jrr = makeRequest(jss::signer_list); + BEAST_EXPECT(checkArraySize(jrr[jss::state], 1)); + for (auto const& j : jrr[jss::state]) + BEAST_EXPECT(j["LedgerEntryType"] == jss::SignerList); + } - { // jvParams[jss::type] = "ticket"; - auto const jrr = makeRequest(jss::ticket); - BEAST_EXPECT(checkArraySize(jrr[jss::state], 1)); - for (auto const& j : jrr[jss::state]) - BEAST_EXPECT(j["LedgerEntryType"] == jss::Ticket); - } + { // jvParams[jss::type] = "state"; + auto const jrr = makeRequest(jss::state); + BEAST_EXPECT(checkArraySize(jrr[jss::state], 1)); + for (auto const& j : jrr[jss::state]) + BEAST_EXPECT(j["LedgerEntryType"] == jss::RippleState); + } - { // jvParams[jss::type] = "escrow"; - auto const jrr = makeRequest(jss::escrow); - BEAST_EXPECT(checkArraySize(jrr[jss::state], 1)); - for (auto const& j : jrr[jss::state]) - BEAST_EXPECT(j["LedgerEntryType"] == jss::Escrow); - } + { // jvParams[jss::type] = "ticket"; + auto const jrr = makeRequest(jss::ticket); + BEAST_EXPECT(checkArraySize(jrr[jss::state], 1)); + for (auto const& j : jrr[jss::state]) + BEAST_EXPECT(j["LedgerEntryType"] == jss::Ticket); + } - { // jvParams[jss::type] = "payment_channel"; - auto const jrr = makeRequest(jss::payment_channel); - BEAST_EXPECT(checkArraySize(jrr[jss::state], 1)); - for (auto const& j : jrr[jss::state]) - BEAST_EXPECT(j["LedgerEntryType"] == jss::PayChannel); - } + { // jvParams[jss::type] = "escrow"; + auto const jrr = makeRequest(jss::escrow); + BEAST_EXPECT(checkArraySize(jrr[jss::state], 1)); + for (auto const& j : jrr[jss::state]) + BEAST_EXPECT(j["LedgerEntryType"] == jss::Escrow); + } - { // jvParams[jss::type] = "deposit_preauth"; - auto const jrr = makeRequest(jss::deposit_preauth); - BEAST_EXPECT(checkArraySize(jrr[jss::state], 2)); - for (auto const& j : jrr[jss::state]) - BEAST_EXPECT(j["LedgerEntryType"] == jss::DepositPreauth); - } + { // jvParams[jss::type] = "payment_channel"; + auto const jrr = makeRequest(jss::payment_channel); + BEAST_EXPECT(checkArraySize(jrr[jss::state], 1)); + for (auto const& j : jrr[jss::state]) + BEAST_EXPECT(j["LedgerEntryType"] == jss::PayChannel); + } - { // jvParams[jss::type] = "misspelling"; - Json::Value jvParams; - jvParams[jss::ledger_index] = "current"; - jvParams[jss::type] = "misspelling"; - auto const jrr = env.rpc( - "json", - "ledger_data", - boost::lexical_cast(jvParams))[jss::result]; - BEAST_EXPECT(jrr.isMember("error")); - BEAST_EXPECT(jrr["error"] == "invalidParams"); - BEAST_EXPECT(jrr["error_message"] == "Invalid field 'type'."); + { // jvParams[jss::type] = "deposit_preauth"; + auto const jrr = makeRequest(jss::deposit_preauth); + BEAST_EXPECT(checkArraySize(jrr[jss::state], 2)); + for (auto const& j : jrr[jss::state]) + BEAST_EXPECT(j["LedgerEntryType"] == jss::DepositPreauth); + } + + { // jvParams[jss::type] = "misspelling"; + Json::Value jvParams; + jvParams[jss::ledger_index] = "current"; + jvParams[jss::type] = "misspelling"; + auto const jrr = env.rpc( + "json", + "ledger_data", + boost::lexical_cast(jvParams))[jss::result]; + BEAST_EXPECT(jrr.isMember("error")); + BEAST_EXPECT(jrr["error"] == "invalidParams"); + BEAST_EXPECT(jrr["error_message"] == "Invalid field 'type'."); + } } } diff --git a/src/test/rpc/Transaction_test.cpp b/src/test/rpc/Transaction_test.cpp index 72a7681ce97..2bd20eb3707 100644 --- a/src/test/rpc/Transaction_test.cpp +++ b/src/test/rpc/Transaction_test.cpp @@ -27,6 +27,7 @@ #include #include +#include #include #include @@ -671,6 +672,47 @@ class Transaction_test : public beast::unit_test::suite BEAST_EXPECT(jrr[jss::hash]); } + // test querying with mixed case ctid + { + Env env{*this, makeNetworkConfig(11111)}; + std::uint32_t const netID = env.app().config().NETWORK_ID; + + Account const alice = Account("alice"); + Account const bob = Account("bob"); + + std::uint32_t const startLegSeq = env.current()->info().seq; + env.fund(XRP(10000), alice, bob); + env(pay(alice, bob, XRP(10))); + env.close(); + + std::string const ctid = *RPC::encodeCTID(startLegSeq, 0, netID); + auto isUpper = [](char c) { return std::isupper(c) != 0; }; + + // Verify that there are at least two upper case letters in ctid and + // test a mixed case + if (BEAST_EXPECT( + std::count_if(ctid.begin(), ctid.end(), isUpper) > 1)) + { + // Change the first upper case letter to lower case. + std::string mixedCase = ctid; + { + auto const iter = std::find_if( + mixedCase.begin(), mixedCase.end(), isUpper); + *iter = std::tolower(*iter); + } + BEAST_EXPECT(ctid != mixedCase); + + Json::Value jsonTx; + jsonTx[jss::binary] = false; + jsonTx[jss::ctid] = mixedCase; + jsonTx[jss::id] = 1; + Json::Value const jrr = + env.rpc("json", "tx", to_string(jsonTx))[jss::result]; + BEAST_EXPECT(jrr[jss::ctid] == ctid); + BEAST_EXPECT(jrr[jss::hash]); + } + } + // test that if the network is 65535 the ctid is not in the response { Env env{*this, makeNetworkConfig(65535)}; diff --git a/src/xrpld/app/ledger/Ledger.cpp b/src/xrpld/app/ledger/Ledger.cpp index afed8f4870b..bcd3b6d4ba7 100644 --- a/src/xrpld/app/ledger/Ledger.cpp +++ b/src/xrpld/app/ledger/Ledger.cpp @@ -793,7 +793,7 @@ Ledger::updateNegativeUNL() if (hasToDisable) { - newNUnl.emplace_back(sfDisabledValidator); + newNUnl.push_back(STObject::makeInnerObject(sfDisabledValidator)); newNUnl.back().setFieldVL( sfPublicKey, sle->getFieldVL(sfValidatorToDisable)); newNUnl.back().setFieldU32(sfFirstLedgerSequence, seq()); diff --git a/src/xrpld/app/misc/FeeVoteImpl.cpp b/src/xrpld/app/misc/FeeVoteImpl.cpp index af57314ef6d..cb4e57b0f73 100644 --- a/src/xrpld/app/misc/FeeVoteImpl.cpp +++ b/src/xrpld/app/misc/FeeVoteImpl.cpp @@ -277,9 +277,9 @@ FeeVoteImpl::doVoting( } // choose our positions - // TODO: Use structured binding once LLVM issue - // https://github.com/llvm/llvm-project/issues/48582 - // is fixed. + // TODO: Use structured binding once LLVM 16 is the minimum supported + // version. See also: https://github.com/llvm/llvm-project/issues/48582 + // https://github.com/llvm/llvm-project/commit/127bf44385424891eb04cff8e52d3f157fc2cb7c auto const baseFee = baseFeeVote.getVotes(); auto const baseReserve = baseReserveVote.getVotes(); auto const incReserve = incReserveVote.getVotes(); diff --git a/src/xrpld/app/misc/detail/AMMUtils.cpp b/src/xrpld/app/misc/detail/AMMUtils.cpp index 0014ab01118..efc80cf17b6 100644 --- a/src/xrpld/app/misc/detail/AMMUtils.cpp +++ b/src/xrpld/app/misc/detail/AMMUtils.cpp @@ -312,7 +312,7 @@ initializeFeeAuctionVote( auto const& rules = view.rules(); // AMM creator gets the voting slot. STArray voteSlots; - STObject voteEntry = STObject::makeInnerObject(sfVoteEntry, rules); + STObject voteEntry = STObject::makeInnerObject(sfVoteEntry); if (tfee != 0) voteEntry.setFieldU16(sfTradingFee, tfee); voteEntry.setFieldU32(sfVoteWeight, VOTE_WEIGHT_SCALE_FACTOR); @@ -325,7 +325,7 @@ initializeFeeAuctionVote( if (rules.enabled(fixInnerObjTemplate) && !ammSle->isFieldPresent(sfAuctionSlot)) { - STObject auctionSlot = STObject::makeInnerObject(sfAuctionSlot, rules); + STObject auctionSlot = STObject::makeInnerObject(sfAuctionSlot); ammSle->set(std::move(auctionSlot)); } STObject& auctionSlot = ammSle->peekFieldObject(sfAuctionSlot); diff --git a/src/xrpld/app/paths/detail/BookStep.cpp b/src/xrpld/app/paths/detail/BookStep.cpp index 4fa6f140aae..96971a516fc 100644 --- a/src/xrpld/app/paths/detail/BookStep.cpp +++ b/src/xrpld/app/paths/detail/BookStep.cpp @@ -23,7 +23,6 @@ #include #include #include -#include #include #include #include diff --git a/src/xrpld/app/tx/detail/AMMVote.cpp b/src/xrpld/app/tx/detail/AMMVote.cpp index 6da320f83e2..c4b6c612c63 100644 --- a/src/xrpld/app/tx/detail/AMMVote.cpp +++ b/src/xrpld/app/tx/detail/AMMVote.cpp @@ -104,7 +104,7 @@ applyVote( Number den{0}; // Account already has vote entry bool foundAccount = false; - auto const& rules = ctx_.view().rules(); + // Iterate over the current vote entries and update each entry // per current total tokens balance and each LP tokens balance. // Find the entry with the least tokens and whether the account @@ -120,7 +120,7 @@ applyVote( continue; } auto feeVal = entry[sfTradingFee]; - STObject newEntry = STObject::makeInnerObject(sfVoteEntry, rules); + STObject newEntry = STObject::makeInnerObject(sfVoteEntry); // The account already has the vote entry. if (account == account_) { @@ -159,7 +159,7 @@ applyVote( { auto update = [&](std::optional const& minPos = std::nullopt) { - STObject newEntry = STObject::makeInnerObject(sfVoteEntry, rules); + STObject newEntry = STObject::makeInnerObject(sfVoteEntry); if (feeNew != 0) newEntry.setFieldU16(sfTradingFee, feeNew); newEntry.setFieldU32( diff --git a/src/xrpld/app/tx/detail/Change.cpp b/src/xrpld/app/tx/detail/Change.cpp index 0ebdb1e93ba..909f35fc799 100644 --- a/src/xrpld/app/tx/detail/Change.cpp +++ b/src/xrpld/app/tx/detail/Change.cpp @@ -300,11 +300,11 @@ Change::applyAmendment() if (gotMajority) { // This amendment now has a majority - newMajorities.push_back(STObject(sfMajority)); + newMajorities.push_back(STObject::makeInnerObject(sfMajority)); auto& entry = newMajorities.back(); - entry.emplace_back(STUInt256(sfAmendment, amendment)); - entry.emplace_back(STUInt32( - sfCloseTime, view().parentCloseTime().time_since_epoch().count())); + entry[sfAmendment] = amendment; + entry[sfCloseTime] = + view().parentCloseTime().time_since_epoch().count(); if (!ctx_.app.getAmendmentTable().isSupported(amendment)) { diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index 59ea9ccbc4d..f855ad8578c 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -358,6 +358,91 @@ AccountRootsNotDeleted::finalize( //------------------------------------------------------------------------------ +void +AccountRootsDeletedClean::visitEntry( + bool isDelete, + std::shared_ptr const& before, + std::shared_ptr const&) +{ + if (isDelete && before && before->getType() == ltACCOUNT_ROOT) + accountsDeleted_.emplace_back(before); +} + +bool +AccountRootsDeletedClean::finalize( + STTx const& tx, + TER const result, + XRPAmount const, + ReadView const& view, + beast::Journal const& j) +{ + // Always check for objects in the ledger, but to prevent differing + // transaction processing results, however unlikely, only fail if the + // feature is enabled. Enabled, or not, though, a fatal-level message will + // be logged + bool const enforce = view.rules().enabled(featureInvariantsV1_1); + + auto const objectExists = [&view, enforce, &j](auto const& keylet) { + if (auto const sle = view.read(keylet)) + { + // Finding the object is bad + auto const typeName = [&sle]() { + auto item = + LedgerFormats::getInstance().findByType(sle->getType()); + + if (item != nullptr) + return item->getName(); + return std::to_string(sle->getType()); + }(); + + JLOG(j.fatal()) + << "Invariant failed: account deletion left behind a " + << typeName << " object"; + (void)enforce; + assert(enforce); + return true; + } + return false; + }; + + for (auto const& accountSLE : accountsDeleted_) + { + auto const accountID = accountSLE->getAccountID(sfAccount); + // Simple types + for (auto const& [keyletfunc, _, __] : directAccountKeylets) + { + if (objectExists(std::invoke(keyletfunc, accountID)) && enforce) + return false; + } + + { + // NFT pages. ntfpage_min and nftpage_max were already explicitly + // checked above as entries in directAccountKeylets. This uses + // view.succ() to check for any NFT pages in between the two + // endpoints. + Keylet const first = keylet::nftpage_min(accountID); + Keylet const last = keylet::nftpage_max(accountID); + + std::optional key = view.succ(first.key, last.key.next()); + + // current page + if (key && objectExists(Keylet{ltNFTOKEN_PAGE, *key}) && enforce) + return false; + } + + // Keys directly stored in the AccountRoot object + if (auto const ammKey = accountSLE->at(~sfAMMID)) + { + if (objectExists(keylet::amm(*ammKey)) && enforce) + return false; + } + } + + return true; +} + +//------------------------------------------------------------------------------ + void LedgerEntryTypesMatch::visitEntry( bool, diff --git a/src/xrpld/app/tx/detail/InvariantCheck.h b/src/xrpld/app/tx/detail/InvariantCheck.h index 55229d9561f..1b3234bae69 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.h +++ b/src/xrpld/app/tx/detail/InvariantCheck.h @@ -164,6 +164,36 @@ class AccountRootsNotDeleted beast::Journal const&); }; +/** + * @brief Invariant: a deleted account must not have any objects left + * + * We iterate all deleted account roots, and ensure that there are no + * objects left that are directly accessible with that account's ID. + * + * There should only be one deleted account, but that's checked by + * AccountRootsNotDeleted. This invariant will handle multiple deleted account + * roots without a problem. + */ +class AccountRootsDeletedClean +{ + std::vector> accountsDeleted_; + +public: + void + visitEntry( + bool, + std::shared_ptr const&, + std::shared_ptr const&); + + bool + finalize( + STTx const&, + TER const, + XRPAmount const, + ReadView const&, + beast::Journal const&); +}; + /** * @brief Invariant: An account XRP balance must be in XRP and take a value * between 0 and INITIAL_XRP drops, inclusive. @@ -425,6 +455,7 @@ class ValidClawback using InvariantChecks = std::tuple< TransactionFeeCheck, AccountRootsNotDeleted, + AccountRootsDeletedClean, LedgerEntryTypesMatch, XRPBalanceChecks, XRPNotCreated, diff --git a/src/xrpld/app/tx/detail/NFTokenBurn.cpp b/src/xrpld/app/tx/detail/NFTokenBurn.cpp index f43dda71f59..725e35791f9 100644 --- a/src/xrpld/app/tx/detail/NFTokenBurn.cpp +++ b/src/xrpld/app/tx/detail/NFTokenBurn.cpp @@ -19,7 +19,6 @@ #include #include -#include #include #include #include diff --git a/src/xrpld/app/tx/detail/NFTokenMint.h b/src/xrpld/app/tx/detail/NFTokenMint.h index e0eb54bbc93..c95fd5944e4 100644 --- a/src/xrpld/app/tx/detail/NFTokenMint.h +++ b/src/xrpld/app/tx/detail/NFTokenMint.h @@ -22,6 +22,7 @@ #include #include +#include namespace ripple { diff --git a/src/xrpld/app/tx/detail/NFTokenUtils.cpp b/src/xrpld/app/tx/detail/NFTokenUtils.cpp index 56588d50da7..3ac6b08c4dd 100644 --- a/src/xrpld/app/tx/detail/NFTokenUtils.cpp +++ b/src/xrpld/app/tx/detail/NFTokenUtils.cpp @@ -18,7 +18,7 @@ //============================================================================== #include -#include +#include #include #include #include @@ -191,6 +191,7 @@ getPageForToken( : carr[0].getFieldH256(sfNFTokenID); auto np = std::make_shared(keylet::nftpage(base, tokenIDForNewPage)); + assert(np->key() > base.key); np->setFieldArray(sfNFTokens, narr); np->setFieldH256(sfNextPageMin, cp->key()); diff --git a/src/xrpld/app/tx/detail/SetSignerList.cpp b/src/xrpld/app/tx/detail/SetSignerList.cpp index 37790f14f40..0949fbbe775 100644 --- a/src/xrpld/app/tx/detail/SetSignerList.cpp +++ b/src/xrpld/app/tx/detail/SetSignerList.cpp @@ -416,11 +416,11 @@ SetSignerList::writeSignersToSLE( STArray toLedger(signers_.size()); for (auto const& entry : signers_) { - toLedger.emplace_back(sfSignerEntry); + toLedger.push_back(STObject::makeInnerObject(sfSignerEntry)); STObject& obj = toLedger.back(); obj.reserve(2); - obj.setAccountID(sfAccount, entry.account); - obj.setFieldU16(sfSignerWeight, entry.weight); + obj[sfAccount] = entry.account; + obj[sfSignerWeight] = entry.weight; // This is a defensive check to make absolutely sure we will never write // a tag into the ledger while featureExpandedSignerList is not enabled diff --git a/src/xrpld/app/tx/detail/SignerEntries.cpp b/src/xrpld/app/tx/detail/SignerEntries.cpp index f6a42120d11..cab362a8e3b 100644 --- a/src/xrpld/app/tx/detail/SignerEntries.cpp +++ b/src/xrpld/app/tx/detail/SignerEntries.cpp @@ -30,7 +30,7 @@ Expected, NotTEC> SignerEntries::deserialize( STObject const& obj, beast::Journal journal, - std::string const& annotation) + std::string_view annotation) { std::pair, NotTEC> s; diff --git a/src/xrpld/app/tx/detail/SignerEntries.h b/src/xrpld/app/tx/detail/SignerEntries.h index c64e106ae9e..2227aa98109 100644 --- a/src/xrpld/app/tx/detail/SignerEntries.h +++ b/src/xrpld/app/tx/detail/SignerEntries.h @@ -27,18 +27,27 @@ #include // STTx::maxMultiSigners #include // temMALFORMED #include // AccountID + #include +#include namespace ripple { // Forward declarations class STObject; -// Support for SignerEntries that is needed by a few Transactors +// Support for SignerEntries that is needed by a few Transactors. +// +// SignerEntries is represented as a std::vector. +// There is no direct constructor for SignerEntries. +// +// o A std::vector is a SignerEntries. +// o More commonly, SignerEntries are extracted from an STObject by +// calling SignerEntries::deserialize(). class SignerEntries { public: - explicit SignerEntries() = default; + explicit SignerEntries() = delete; struct SignerEntry { @@ -69,11 +78,15 @@ class SignerEntries }; // Deserialize a SignerEntries array from the network or from the ledger. + // + // obj Contains a SignerEntries field that is an STArray. + // journal For reporting error conditions. + // annotation Source of SignerEntries, like "ledger" or "transaction". static Expected, NotTEC> deserialize( STObject const& obj, beast::Journal journal, - std::string const& annotation); + std::string_view annotation); }; } // namespace ripple diff --git a/src/xrpld/ledger/Directory.h b/src/xrpld/ledger/Dir.h similarity index 86% rename from src/xrpld/ledger/Directory.h rename to src/xrpld/ledger/Dir.h index cc8b3c13a3a..0e92d7dbca6 100644 --- a/src/xrpld/ledger/Directory.h +++ b/src/xrpld/ledger/Dir.h @@ -25,6 +25,18 @@ namespace ripple { +/** A class that simplifies iterating ledger directory pages + + The Dir class provides a forward iterator for walking through + the uint256 values contained in ledger directories. + + The Dir class also allows accelerated directory walking by + stepping directly from one page to the next using the next_page() + member function. + + As of July 2024, the Dir class is only being used with NFTokenOffer + directories and for unit tests. +*/ class Dir { private: diff --git a/src/xrpld/ledger/detail/Directory.cpp b/src/xrpld/ledger/detail/Dir.cpp similarity index 98% rename from src/xrpld/ledger/detail/Directory.cpp rename to src/xrpld/ledger/detail/Dir.cpp index 9153d013fd8..6004a73095c 100644 --- a/src/xrpld/ledger/detail/Directory.cpp +++ b/src/xrpld/ledger/detail/Dir.cpp @@ -17,7 +17,7 @@ */ //============================================================================== -#include +#include namespace ripple { diff --git a/src/xrpld/rpc/CTID.h b/src/xrpld/rpc/CTID.h index 8f6c64bc028..8cac8d63171 100644 --- a/src/xrpld/rpc/CTID.h +++ b/src/xrpld/rpc/CTID.h @@ -63,7 +63,7 @@ decodeCTID(const T ctid) noexcept if (ctidString.length() != 16) return {}; - if (!boost::regex_match(ctidString, boost::regex("^[0-9A-F]+$"))) + if (!boost::regex_match(ctidString, boost::regex("^[0-9A-Fa-f]+$"))) return {}; ctidValue = std::stoull(ctidString, nullptr, 16); diff --git a/src/xrpld/rpc/detail/RPCHelpers.cpp b/src/xrpld/rpc/detail/RPCHelpers.cpp index 42bd4faded5..71513ddcd5c 100644 --- a/src/xrpld/rpc/detail/RPCHelpers.cpp +++ b/src/xrpld/rpc/detail/RPCHelpers.cpp @@ -986,6 +986,22 @@ chooseLedgerEntryType(Json::Value const& params) return result; } +bool +isAccountObjectsValidType(LedgerEntryType const& type) +{ + switch (type) + { + case LedgerEntryType::ltAMENDMENTS: + case LedgerEntryType::ltDIR_NODE: + case LedgerEntryType::ltFEE_SETTINGS: + case LedgerEntryType::ltLEDGER_HASHES: + case LedgerEntryType::ltNEGATIVE_UNL: + return false; + default: + return true; + } +} + beast::SemanticVersion const firstVersion("1.0.0"); beast::SemanticVersion const goodVersion("1.0.0"); beast::SemanticVersion const lastVersion("1.0.0"); diff --git a/src/xrpld/rpc/detail/RPCHelpers.h b/src/xrpld/rpc/detail/RPCHelpers.h index 30b48807fcd..54c426b17c3 100644 --- a/src/xrpld/rpc/detail/RPCHelpers.h +++ b/src/xrpld/rpc/detail/RPCHelpers.h @@ -232,6 +232,15 @@ setVersion(Object& parent, unsigned int apiVersion, bool betaEnabled) std::pair chooseLedgerEntryType(Json::Value const& params); +/** + * Check if the type is a valid filtering type for account_objects method + * + * Since Amendments, DirectoryNode, FeeSettings, LedgerHashes can not be + * owned by an account, this function will return false in these situations. + */ +bool +isAccountObjectsValidType(LedgerEntryType const& type); + /** * Retrieve the api version number from the json value * diff --git a/src/xrpld/rpc/detail/TransactionSign.cpp b/src/xrpld/rpc/detail/TransactionSign.cpp index 5ea895b60e4..1fee84c683b 100644 --- a/src/xrpld/rpc/detail/TransactionSign.cpp +++ b/src/xrpld/rpc/detail/TransactionSign.cpp @@ -1051,7 +1051,7 @@ transactionSignFor( auto& sttx = preprocResult.second; { // Make the signer object that we'll inject. - STObject signer(sfSigner); + STObject signer = STObject::makeInnerObject(sfSigner); signer[sfAccount] = *signerAccountID; signer.setFieldVL(sfTxnSignature, signForParams.getSignature()); signer.setFieldVL( diff --git a/src/xrpld/rpc/handlers/AccountObjects.cpp b/src/xrpld/rpc/handlers/AccountObjects.cpp index dfc4e9b3388..d2eb2eacde6 100644 --- a/src/xrpld/rpc/handlers/AccountObjects.cpp +++ b/src/xrpld/rpc/handlers/AccountObjects.cpp @@ -75,8 +75,9 @@ doAccountNFTs(RPC::JsonContext& context) return *err; uint256 marker; + bool const markerSet = params.isMember(jss::marker); - if (params.isMember(jss::marker)) + if (markerSet) { auto const& m = params[jss::marker]; if (!m.isString()) @@ -98,6 +99,7 @@ doAccountNFTs(RPC::JsonContext& context) // Continue iteration from the current page: bool pastMarker = marker.isZero(); + bool markerFound = false; uint256 const maskedMarker = marker & nft::pageMask; while (cp) { @@ -119,12 +121,23 @@ doAccountNFTs(RPC::JsonContext& context) uint256 const nftokenID = o[sfNFTokenID]; uint256 const maskedNftokenID = nftokenID & nft::pageMask; - if (!pastMarker && maskedNftokenID < maskedMarker) - continue; + if (!pastMarker) + { + if (maskedNftokenID < maskedMarker) + continue; - if (!pastMarker && maskedNftokenID == maskedMarker && - nftokenID <= marker) - continue; + if (maskedNftokenID == maskedMarker && nftokenID < marker) + continue; + + if (nftokenID == marker) + { + markerFound = true; + continue; + } + } + + if (markerSet && !markerFound) + return RPC::invalid_field_error(jss::marker); pastMarker = true; @@ -155,6 +168,9 @@ doAccountNFTs(RPC::JsonContext& context) cp = nullptr; } + if (markerSet && !markerFound) + return RPC::invalid_field_error(jss::marker); + result[jss::account] = toBase58(accountID); context.loadType = Resource::feeMediumBurdenRPC; return result; @@ -220,6 +236,9 @@ doAccountObjects(RPC::JsonContext& context) { auto [rpcStatus, type] = RPC::chooseLedgerEntryType(params); + if (!RPC::isAccountObjectsValidType(type)) + return RPC::invalid_field_error(jss::type); + if (rpcStatus) { result.clear(); diff --git a/src/xrpld/rpc/handlers/Feature1.cpp b/src/xrpld/rpc/handlers/Feature1.cpp index d4499f120ef..c06756ca00a 100644 --- a/src/xrpld/rpc/handlers/Feature1.cpp +++ b/src/xrpld/rpc/handlers/Feature1.cpp @@ -38,6 +38,15 @@ doFeature(RPC::JsonContext& context) if (context.app.config().reporting()) return rpcError(rpcREPORTING_UNSUPPORTED); + if (context.params.isMember(jss::feature)) + { + // ensure that the `feature` param is a string + if (!context.params[jss::feature].isString()) + { + return rpcError(rpcINVALID_PARAMS); + } + } + bool const isAdmin = context.role == Role::ADMIN; // Get majority amendment status majorityAmendments_t majorities; diff --git a/src/xrpld/shamap/detail/TaggedPointer.h b/src/xrpld/shamap/detail/TaggedPointer.h index 58271f59c01..48534076548 100644 --- a/src/xrpld/shamap/detail/TaggedPointer.h +++ b/src/xrpld/shamap/detail/TaggedPointer.h @@ -37,7 +37,7 @@ namespace ripple { low bits. When dereferencing the pointer, these low "tag" bits are set to zero. When accessing the tag bits, the high "pointer" bits are set to zero. - The "pointer" part points to to the equivalent to an array of + The "pointer" part points to the equivalent to an array of `SHAMapHash` followed immediately by an array of `shared_ptr`. The sizes of these arrays are determined by the tag. The tag is an index into an array (`boundaries`, diff --git a/src/xrpld/shamap/detail/TaggedPointer.ipp b/src/xrpld/shamap/detail/TaggedPointer.ipp index 6770b53021e..309913c79c0 100644 --- a/src/xrpld/shamap/detail/TaggedPointer.ipp +++ b/src/xrpld/shamap/detail/TaggedPointer.ipp @@ -258,7 +258,7 @@ TaggedPointer::getChildIndex(std::uint16_t isBranch, int i) const // of a child in the array is the number of non-empty children // before it. Since `isBranch_` is a bitset of the stored // children, we simply need to mask out (and set to zero) all - // the bits in `isBranch_` equal to to higher than `i` and count + // the bits in `isBranch_` equal to higher than `i` and count // the bits. // mask sets all the bits >=i to zero and all the bits