aboutsummaryrefslogtreecommitdiff
path: root/src/rpc
AgeCommit message (Collapse)Author
2024-03-13Merge bitcoin/bitcoin#27375: net: support unix domain sockets for -proxy and ↵Ava Chow
-onion 567cec9a05e1261e955535f734826b12341684b6 doc: add release notes and help text for unix sockets (Matthew Zipkin) bfe51928911daf484ae07deb52a7ff0bcb2526ae test: cover UNIX sockets in feature_proxy.py (Matthew Zipkin) c65c0d01630b44fa71321ea7ad68d5f9fbb7aefb init: allow UNIX socket path for -proxy and -onion (Matthew Zipkin) c3bd43142eba77dcf1acd4984e437759f65e237a gui: accomodate unix socket Proxy in updateDefaultProxyNets() (Matthew Zipkin) a88bf9dedd1d8c1db0a9c8b663dab3e3c2f0f030 i2p: construct Session with Proxy instead of CService (Matthew Zipkin) d9318a37ec09fe0b002815a7e48710e530620ae2 net: split ConnectToSocket() from ConnectDirectly() for unix sockets (Matthew Zipkin) ac2ecf3182fb5ad9bcd41540b19382376114d6ee proxy: rename randomize_credentials to m_randomize_credentials (Matthew Zipkin) a89c3f59dc44eaf4f59912c1accfc0ce5d61933a netbase: extend Proxy class to wrap UNIX socket as well as TCP (Matthew Zipkin) 3a7d6548effa6cd9a4a5413b690c2fd85da4ef65 net: move CreateSock() calls from ConnectNode() to netbase methods (Matthew Zipkin) 74f568cb6fd5c74b7b9bf0ce69876430746a53b1 netbase: allow CreateSock() to create UNIX sockets if supported (Matthew Zipkin) bae86c8d318d06818aa75a9ebe3db864197f0bc6 netbase: refactor CreateSock() to accept sa_family_t (Matthew Zipkin) adb3a3e51de205cc69b1a58647c65c04fa6c6362 configure: test for unix domain sockets (Matthew Zipkin) Pull request description: Closes https://github.com/bitcoin/bitcoin/issues/27252 UNIX domain sockets are a mechanism for inter-process communication that are faster than local TCP ports (because there is no need for TCP overhead) and potentially more secure because access is managed by the filesystem instead of serving an open port on the system. There has been work on [unix domain sockets before](https://github.com/bitcoin/bitcoin/pull/9979) but for now I just wanted to start on this single use-case which is enabling unix sockets from the client side, specifically connecting to a local Tor proxy (Tor can listen on unix sockets and even enforces strict curent-user-only access permission before binding) configured by `-onion=` or `-proxy=` I copied the prefix `unix:` usage from Tor. With this patch built locally you can test with your own filesystem path (example): `tor --SocksPort unix:/Users/matthewzipkin/torsocket/x` `bitcoind -proxy=unix:/Users/matthewzipkin/torsocket/x` Prep work for this feature includes: - Moving where and how we create `sockaddr` and `Sock` to accommodate `AF_UNIX` without disturbing `CService` - Expanding `Proxy` class to represent either a `CService` or a UNIX socket (by its file path) Future work: - Enable UNIX sockets for ZMQ (https://github.com/bitcoin/bitcoin/pull/27679) - Enable UNIX sockets for I2P SAM proxy (some code is included in this PR but not tested or exposed to user options yet) - Enable UNIX sockets on windows where supported - Update Network Proxies dialog in GUI to support UNIX sockets ACKs for top commit: Sjors: re-ACK 567cec9a05e1261e955535f734826b12341684b6 tdb3: re ACK for 567cec9a05e1261e955535f734826b12341684b6. achow101: ACK 567cec9a05e1261e955535f734826b12341684b6 vasild: ACK 567cec9a05e1261e955535f734826b12341684b6 Tree-SHA512: de81860e56d5de83217a18df4c35297732b4ad491e293a0153d2d02a0bde1d022700a1131279b187ef219651487537354b9d06d10fde56225500c7e257df92c1
2024-03-12rpc: read raw block in getblock and deserialize for verbosity > 0Andrew Toth
Note that for speed this commit also removes the proof of work and signet signature checks before returning the block in getblock. It is assumed if a block is stored it will be valid.
2024-03-08Merge bitcoin/bitcoin#28960: kernel: Remove dependency on CSchedulerAva Chow
d5228efb5391b31a9a0673019e43e7fa2cd4ac07 kernel: Remove dependency on CScheduler (TheCharlatan) 06069b3913dda048f5d640a662b0852f86346ace scripted-diff: Rename MainSignals to ValidationSignals (TheCharlatan) 0d6d2b650d1017691f48c9109a6cd020ab46aa73 scripted-diff: Rename SingleThreadedSchedulerClient to SerialTaskRunner (TheCharlatan) 4abde2c4e3fd9b66394b79874583bdc2a9132c36 [refactor] Make MainSignals RAII styled (TheCharlatan) 84f5c135b8118cbe15b8bfb4db80d61237987f64 refactor: De-globalize g_signals (TheCharlatan) 473dd4b97ae40e43e1a1a97fdbeb40be4855e9bc [refactor] Prepare for g_signals de-globalization (TheCharlatan) 3fba3d5deec6d7bae33823b8da7682f9b03d9deb [refactor] Make signals optional in mempool and chainman (TheCharlatan) Pull request description: By defining a virtual interface class for the scheduler client, users of the kernel can now define their own event consuming infrastructure, without having to spawn threads or rely on the scheduler design. Removing `CScheduler` also allows removing the thread and exception modules from the kernel library. To make the `CMainSignals` class easier to use from a kernel library perspective, remove its global instantiation and adopt RAII practices. Renames `CMainSignals` to `ValidationSignals`, which more accurately describes its purpose and scope. Also make the `ValidationSignals` in the `ChainstateManager` and CTxMemPool` optional. This could be useful in the future for using or testing these classes without having to instantiate any form of signal handling. --- This PR is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587). It improves the kernel API and removes two modules from the kernel library. ACKs for top commit: maflcko: re-ACK d5228efb5391b31a9a0673019e43e7fa2cd4ac07 🌄 ryanofsky: Code review ACK d5228efb5391b31a9a0673019e43e7fa2cd4ac07. Just comment change since last review. vasild: ACK d5228efb5391b31a9a0673019e43e7fa2cd4ac07 furszy: diff ACK d5228ef Tree-SHA512: e93a5f10eb6182effb84bb981859a7ce750e466efd8171045d8d9e7fe46e4065631d9f6f533c5967c4d34c9bb7d7a67e9f4593bd4c5b30cd7b3bbad7be7b331b
2024-03-01proxy: rename randomize_credentials to m_randomize_credentialsMatthew Zipkin
2024-03-01netbase: extend Proxy class to wrap UNIX socket as well as TCPMatthew Zipkin
2024-02-26Merge bitcoin/bitcoin#29345: rpc: Do not wait for headers inside loadtxoutsetfanquake
faa30a4c566c5b720c7994c55f276352a119129f rpc: Do not wait for headers inside loadtxoutset (MarcoFalke) Pull request description: While the `loadtxoutset` default 10 minute timeout is convenient when it is sufficient, it may cause hassle where it is not. For example: * When P2P connections are missing, it seems better to abort early than wait for the timeout. * When the 10 minute timeout is not sufficient, the RPC will have to be called again, so a check or loop is needed outside the RPC either way. So might as well remove the loop inside the RPC. ACKs for top commit: fjahr: ACK faa30a4c56 theStack: Code-review ACK faa30a4c566c5b720c7994c55f276352a119129f pablomartin4btc: tACK faa30a4c566c5b720c7994c55f276352a119129f TheCharlatan: ACK faa30a4c566c5b720c7994c55f276352a119129f Tree-SHA512: 9167c7d8b2889bb3fd369de4acd2cc4d24a2fe225018d82bd9568ecd737093f6e19be7cc62815b574137b61076a6f773c29bff75398991b5cd702423aab2322b
2024-02-20Merge bitcoin/bitcoin#29404: refactor: bitcoin-config.h includes cleanupfanquake
9d1dbbd4ceb8c04340927f5127195dc306adf3fc scripted-diff: Fix bitcoin_config_h includes (TheCharlatan) Pull request description: As mentioned in https://github.com/bitcoin/bitcoin/pull/26924#issuecomment-1403449932 and https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1922334399, it is currently not safe to remove `bitcoin-config.h` includes from headers because some unrelated file might be depending on it. See also #26972 for discussion. Solve this by including the file directly everywhere it's required, regardless of whether or not it's already included by another header. There should be no functional change here, but it will allow us to safely remove includes from headers in the future. ~I'm afraid it's a bit tedious to reproduce these commits, but it's reasonably straightforward:~ Edit: See note below ```bash # All commands executed from the src/ subdir. # Collect all tokens from bitcoin-config.h.in # Isolate the tokens and remove blank lines # Replace newlines with | and remove the last trailing one # Collect all files which use these tokens # Filter out subprojects (proper forwarding can be verified from Makefiles) # Filter out .rc files # Save to a text file git grep -E -l `grep undef config/bitcoin-config.h.in | cut -d" " -f2 | grep -v '^$' | tr '\n' '|' | sed 's/|$//'` | grep -v -e "^leveldb/" -e "^secp256k1/" -e "^crc32c/" -e "^minisketch/" -e "^Makefile" -e "\.rc$" > files-with-config-include.txt # Find all files from the above list which don't include bitcoin-config.h git grep -L -E "config/bitcoin-config.h" -- `cat files-with-config-include.txt` # Include them manually with the exception of some files in crypto: # crypto/sha256_arm_shani.cpp crypto/sha256_avx2.cpp crypto/sha256_sse41.cpp crypto/sha256_x86_shani.cpp # These are exceptions which don't use bitcoin-config.h, rather the Makefile.am adds these cppflags manually. # Commit changes. This should match the first commit of this PR. # Use the same search as above to find all files which DON'T use any config tokens git grep -E -L `grep undef config/bitcoin-config.h.in | cut -d" " -f2 | grep -v '^$' | tr '\n' '|' | sed 's/|$//'` | grep -v -e "^leveldb/" -e "^secp256k1/" -e "^crc32c/" -e "^minisketch/" -e "^Makefile" -e "\.rc$" > files-without-config-include.txt # Manually remove the includes and commit changes. This should match the second commit of this PR. ``` Edit: I'll keep this old description for posterity, but the manual approach has been replaced with a scripted diff from TheCharlatan ACKs for top commit: maflcko: ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3f 🚪 TheCharlatan: ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3fc hebasto: ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3fc, I have reviewed the code and it looks OK. fanquake: ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3fc Tree-SHA512: f11ddc4ae6a887f96b954a6b77f310558ddb271088a3fda3edc833669c4251b7f392515224bbb8e5f67eb2c799b4ffed3b07d96454e82ec635c686d0df545872
2024-02-15doc: Clarify maxfeerate helpMarcoFalke
2024-02-15refactor: De-globalize g_signalsTheCharlatan
2024-02-15rpc: Fixed signed integer overflow for large feeratesMarcoFalke
2024-02-15rpc: Add ParseFeeRate helperMarcoFalke
2024-02-14rpc: Implement RPCHelpMan::ArgValue<> for UniValueMarcoFalke
2024-02-13scripted-diff: Fix bitcoin_config_h includesTheCharlatan
-BEGIN VERIFY SCRIPT- regex_string='^(?!//).*(AC_APPLE_UNIVERSAL_BUILD|BOOST_PROCESS_USE_STD_FS|CHAR_EQUALS_INT8|CLIENT_VERSION_BUILD|CLIENT_VERSION_IS_RELEASE|CLIENT_VERSION_MAJOR|CLIENT_VERSION_MINOR|COPYRIGHT_HOLDERS|COPYRIGHT_HOLDERS_FINAL|COPYRIGHT_HOLDERS_SUBSTITUTION|COPYRIGHT_YEAR|ENABLE_ARM_SHANI|ENABLE_AVX2|ENABLE_EXTERNAL_SIGNER|ENABLE_SSE41|ENABLE_TRACING|ENABLE_WALLET|ENABLE_X86_SHANI|ENABLE_ZMQ|HAVE_BOOST|HAVE_BUILTIN_CLZL|HAVE_BUILTIN_CLZLL|HAVE_BYTESWAP_H|HAVE_CLMUL|HAVE_CONSENSUS_LIB|HAVE_CXX20|HAVE_DECL_BE16TOH|HAVE_DECL_BE32TOH|HAVE_DECL_BE64TOH|HAVE_DECL_BSWAP_16|HAVE_DECL_BSWAP_32|HAVE_DECL_BSWAP_64|HAVE_DECL_FORK|HAVE_DECL_FREEIFADDRS|HAVE_DECL_GETIFADDRS|HAVE_DECL_HTOBE16|HAVE_DECL_HTOBE32|HAVE_DECL_HTOBE64|HAVE_DECL_HTOLE16|HAVE_DECL_HTOLE32|HAVE_DECL_HTOLE64|HAVE_DECL_LE16TOH|HAVE_DECL_LE32TOH|HAVE_DECL_LE64TOH|HAVE_DECL_PIPE2|HAVE_DECL_SETSID|HAVE_DECL_STRERROR_R|HAVE_DEFAULT_VISIBILITY_ATTRIBUTE|HAVE_DLFCN_H|HAVE_DLLEXPORT_ATTRIBUTE|HAVE_ENDIAN_H|HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR|HAVE_FDATASYNC|HAVE_GETENTROPY_RAND|HAVE_GETRANDOM|HAVE_GMTIME_R|HAVE_INTTYPES_H|HAVE_LIBADVAPI32|HAVE_LIBCOMCTL32|HAVE_LIBCOMDLG32|HAVE_LIBGDI32|HAVE_LIBIPHLPAPI|HAVE_LIBKERNEL32|HAVE_LIBOLE32|HAVE_LIBOLEAUT32|HAVE_LIBSHELL32|HAVE_LIBSHLWAPI|HAVE_LIBUSER32|HAVE_LIBUUID|HAVE_LIBWINMM|HAVE_LIBWS2_32|HAVE_MALLOC_INFO|HAVE_MALLOPT_ARENA_MAX|HAVE_MINIUPNPC_MINIUPNPC_H|HAVE_MINIUPNPC_UPNPCOMMANDS_H|HAVE_MINIUPNPC_UPNPERRORS_H|HAVE_NATPMP_H|HAVE_O_CLOEXEC|HAVE_POSIX_FALLOCATE|HAVE_PTHREAD|HAVE_PTHREAD_PRIO_INHERIT|HAVE_STDINT_H|HAVE_STDIO_H|HAVE_STDLIB_H|HAVE_STRERROR_R|HAVE_STRINGS_H|HAVE_STRING_H|HAVE_STRONG_GETAUXVAL|HAVE_SYSCTL|HAVE_SYSCTL_ARND|HAVE_SYSTEM|HAVE_SYS_ENDIAN_H|HAVE_SYS_PRCTL_H|HAVE_SYS_RESOURCES_H|HAVE_SYS_SELECT_H|HAVE_SYS_STAT_H|HAVE_SYS_SYSCTL_H|HAVE_SYS_TYPES_H|HAVE_SYS_VMMETER_H|HAVE_THREAD_LOCAL|HAVE_TIMINGSAFE_BCMP|HAVE_UNISTD_H|HAVE_VM_VM_PARAM_H|LT_OBJDIR|PACKAGE_BUGREPORT|PACKAGE_NAME|PACKAGE_STRING|PACKAGE_TARNAME|PACKAGE_URL|PACKAGE_VERSION|PTHREAD_CREATE_JOINABLE|QT_QPA_PLATFORM_ANDROID|QT_QPA_PLATFORM_COCOA|QT_QPA_PLATFORM_MINIMAL|QT_QPA_PLATFORM_WINDOWS|QT_QPA_PLATFORM_XCB|QT_STATICPLUGIN|STDC_HEADERS|STRERROR_R_CHAR_P|USE_ASM|USE_BDB|USE_DBUS|USE_NATPMP|USE_QRCODE|USE_SQLITE|USE_UPNP|_FILE_OFFSET_BITS|_LARGE_FILES)' exclusion_files=":(exclude)src/minisketch :(exclude)src/crc32c :(exclude)src/secp256k1 :(exclude)src/crypto/sha256_arm_shani.cpp :(exclude)src/crypto/sha256_avx2.cpp :(exclude)src/crypto/sha256_sse41.cpp :(exclude)src/crypto/sha256_x86_shani.cpp" git grep --perl-regexp --files-with-matches "$regex_string" -- '*.cpp' $exclusion_files | xargs git grep -L "bitcoin-config.h" | while read -r file; do line_number=$(awk -v my_file="$file" '/\/\/ file COPYING or https?:\/\/www.opensource.org\/licenses\/mit-license.php\./ {line = NR} /^\/\// && NR == line + 1 {while(getline && /^\/\//) line = NR} END {print line+1}' "$file"); sed -i "${line_number}i\\\\n\#if defined(HAVE_CONFIG_H)\\n#include <config/bitcoin-config.h>\\n\#endif" "$file"; done; git grep --perl-regexp --files-with-matches "$regex_string" -- '*.h' $exclusion_files | xargs git grep -L "bitcoin-config.h" | while read -r file; do sed -i "/#define.*_H/a \\\\n\#if defined(HAVE_CONFIG_H)\\n#include <config/bitcoin-config.h>\\n\#endif" "$file"; done; for file in $(git grep --files-with-matches 'bitcoin-config.h' -- '*.cpp' '*.h' $exclusion_files); do if ! grep -q --perl-regexp "$regex_string" $file; then sed -i '/HAVE_CONFIG_H/{N;N;N;d;}' $file; fi; done; -END VERIFY SCRIPT- The first command creates a regular expression for matching all bitcoin-config.h symbols in the following form: ^(?!//).*(AC_APPLE_UNIVERSAL_BUILD|BOOST_PROCESS_USE_STD_FS|...|_LARGE_FILES). It was generated with: ./autogen.sh && printf '^(?!//).*(%s)' $(awk '/^#undef/ {print $2}' src/config/bitcoin-config.h.in | paste -sd "|" -) The second command holds a list of files and directories that should not be processed. These include subtree directories as well as some crypto files that already get their symbols through the makefile. The third command checks for missing bitcoin-config headers in .cpp files and adds the header if it is missing. The fourth command checks for missing bitcoin-config headers in .h files and adds the header if it is missing. The fifth command checks for unneeded bitcoin-config headers in sources files and removes the header if it is unneeded.
2024-02-09Merge bitcoin/bitcoin#28948: v3 transaction policy for anti-pinningAva Chow
29029df5c700e6940c712028303761d91ae15847 [doc] v3 signaling in mempool-replacements.md (glozow) e643ea795e4b6fea4a6bbb3d72870ee6a4c836b1 [fuzz] v3 transactions and sigop-adjusted vsize (glozow) 1fd16b5c62f54c7f4c60122acd65d852f63d1e8b [functional test] v3 transaction submission (glozow) 27c8786ba918a42c860e6a50eaee9fdf56d7c646 test framework: Add and use option for tx-version in MiniWallet methods (MarcoFalke) 9a1fea55b29fe025355b06b45e3d77d192acc635 [policy/validation] allow v3 transactions with certain restrictions (glozow) eb8d5a2e7d939dd3ee683486e98702079e0dfcc0 [policy] add v3 policy rules (glozow) 9a29d470fbb62bbb27d517efeafe46ff03c25f54 [rpc] return full string for package_msg and package-error (glozow) 158623b8e0726dff7eae4288138f1710e727db9c [refactor] change Workspace::m_conflicts and adjacent funcs/structs to use Txid (glozow) Pull request description: See #27463 for overall package relay tracking. Delving Bitcoin discussion thread: https://delvingbitcoin.org/t/v3-transaction-policy-for-anti-pinning/340 Delving Bitcoin discussion for LN usage: https://delvingbitcoin.org/t/lightning-transactions-with-v3-and-ephemeral-anchors/418 Rationale: - There are various pinning problems with RBF and our general ancestor/descendant limits. These policies help mitigate many pinning attacks and make package RBF feasible (see #28984 which implements package RBF on top of this). I would focus the most here on Rule 3 pinning. [1][2] - Switching to a cluster-based mempool (see #27677 and #28676) requires the removal of CPFP carve out, which applications depend on. V3 + package RBF + ephemeral anchors + 1-parent-1-child package relay provides an intermediate solution. V3 policy is for "Priority Transactions." [3][4] It allows users to opt in to more restrictive topological limits for shared transactions, in exchange for the more robust fee-bumping abilities that offers. Even though we don't have cluster limits, we are able to treat these transactions as having as having a maximum cluster size of 2. Immediate benefits: - You can presign a transaction with 0 fees (not just 1sat/vB!) and add a fee-bump later. - Rule 3 pinning is reduced by a significant amount, since the attacker can only attach a maximum of 1000vB to your shared transaction. This also enables some other cool things (again see #27463 for overall roadmap): - Ephemeral Anchors - Package RBF for these 1-parent-1-child packages. That means e.g. a commitment tx + child can replace another commitment tx using the child's fees. - We can transition to a "single anchor" universe without worrying about package limit pinning. So current users of CPFP carve out would have something else to use. - We can switch to a cluster-based mempool [5] (#27677 #28676), which removes CPFP carve out [6]. [1]: Original mailing list post and discussion about RBF pinning problems https://gist.github.com/glozow/25d9662c52453bd08b4b4b1d3783b9ff, https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-January/019817.html [2]: A FAQ is "we need this for cluster mempool, but is this still necessary afterwards?" There are some pinning issues that are fixed here and not fully fixed in cluster mempool, so we will still want this or something similar afterward. [3]: Mailing list post for v3 https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-September/020937.html [4]: Original PR #25038 also contains a lot of the discussion [5]: https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393/7 [6]: https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393#the-cpfp-carveout-rule-can-no-longer-be-supported-12 ACKs for top commit: sdaftuar: ACK 29029df5c700e6940c712028303761d91ae15847 achow101: ACK 29029df5c700e6940c712028303761d91ae15847 instagibbs: ACK 29029df5c700e6940c712028303761d91ae15847 modulo that Tree-SHA512: 9664b078890cfdca2a146439f8835c9d9ab483f43b30af8c7cd6962f09aa557fb1ce7689d5e130a2ec142235dbc8f21213881baa75241c5881660f9008d68450
2024-02-06Merge bitcoin/bitcoin#29356: test: make v2transport arg in addconnection ↵glozow
mandatory and few cleanups e7fd70f4b6b163f4ad5b25b4da7fa79899245235 [test] make v2transport arg in addconnection mandatory and few cleanups (stratospher) Pull request description: - make `v2transport` argument in `addconnection` regression-testing only RPC mandatory. https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1470738750 - previously it was an optional arg with default `false` value. - only place this RPC is used is in the [functional tests](https://github.com/bitcoin/bitcoin/blob/11b436a66af3ceaebb0f907878715f331516a0bc/test/functional/test_framework/test_node.py#L742) where we always pass the appropriate `v2transport` option to the RPC anyways. (and that too just for python dummy peer(`P2PInterface`) and bitcoind(`TestNode`) interactions) - rename `v2_handshake()` to `_on_data_v2_handshake()` https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1466958424 - more compact return statement in `wait_for_reconnect()` https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1466979708 - assertion to check that empty version packets are received from `TestNode`. ACKs for top commit: glozow: ACK e7fd70f4b6 theStack: Code-review ACK e7fd70f4b6b163f4ad5b25b4da7fa79899245235 mzumsande: Code Review ACK e7fd70f4b6b163f4ad5b25b4da7fa79899245235 Tree-SHA512: e66e29baccd91e1e4398b91f7d45c5fc7c2841d77d8a6178734586017bf2be63496721649da91848dec71da605ee31664352407d5bb896e624cc693767c61a1f
2024-02-01refactor: Fix timedata includesMarcoFalke
2024-01-31Merge bitcoin/bitcoin#28956: Nuke adjusted time from validation (attempt 2)Ava Chow
ff9039f6ea876bab2c40a06a93e0dd087f445fa2 Remove GetAdjustedTime (dergoegge) Pull request description: This picks up parts of #25908. The use of adjusted time is removed from validation code while the warning to users if their clock is out of sync with the rest of the network remains. ACKs for top commit: naumenkogs: ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2 achow101: ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2 maflcko: lgtm ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2 🤽 stickies-v: ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2 Tree-SHA512: d1f6b9445c236915503fd2ea828f0d3b92285a5dbc677b168453276115e349972edbad37194d8becd9136d8e7219b576af64ec51c72bdb1923e57e405c0483fc
2024-01-31[test] make v2transport arg in addconnection mandatory and few cleanupsstratospher
`TestNode::add_outbound_p2p_connection()` is the only place where addconnection test-only RPC is used. here, we always pass the appropriate v2transport option to addconnection RPC. currently the v2transport option for addconnection RPC is optional. so simply make the v2transport option mandatory instead.
2024-01-30rpc: Do not wait for headers inside loadtxoutsetMarcoFalke
2024-01-29Merge bitcoin/bitcoin#24748: test/BIP324: functional tests for v2 P2P encryptionAva Chow
bc9283c4415a932ec1eeb70ca2aa4399c80437b3 [test] Add functional test to test early key response behaviour in BIP 324 (stratospher) ffe6a56d75c0b47d0729e4e0b7225a827b43ad89 [test] Check whether v2 TestNode performs downgrading (stratospher) ba737358a37438c18f0fba723eab10ccfd9aae9b [test] Add functional tests to test v2 P2P behaviour (stratospher) 4115cf995647d1a513caecb54a4ff3f51927aa8e [test] Ignore BIP324 decoy messages (stratospher) 8c054aa04d33b247744b3747cd5bf3005a013e90 [test] Allow inbound and outbound connections supporting v2 P2P protocol (stratospher) 382894c3acd2dbf3e4198814f547c75b6fb17706 [test] Reconnect using v1 P2P when v2 P2P terminates due to magic byte mismatch (stratospher) a94e350ac0e5b65ef23a84b05fb10d1204c98c97 [test] Build v2 P2P messages (stratospher) bb7bffed799dc5ad8b606768164fce46d4cbf9d0 [test] Use lock for sending P2P messages in test framework (stratospher) 5b91fb14aba7d7fe45c9ac364526815bec742356 [test] Read v2 P2P messages (stratospher) 05bddb20f5cc9036fd680500bde8ece70dbf0646 [test] Perform initial v2 handshake (stratospher) a049d1bd08c8cdb3b693520f24f8a82572dcaab1 [test] Introduce EncryptedP2PState object in P2PConnection (stratospher) b89fa59e715a185d9fa7fce089dad4273d3b1532 [test] Construct class to handle v2 P2P protocol functions (stratospher) 8d6c848a48530893ca40be5c1285541b3e7a94f3 [test] Move MAGIC_BYTES to messages.py (stratospher) 595ad4b16880ae1f23463ca9985381c8eae945d8 [test/crypto] Add ECDH (stratospher) 4487b8051797173c7ab432e75efa370afb03b529 [rpc/net] Allow v2 p2p support in addconnection (stratospher) Pull request description: This PR introduces support for v2 P2P encryption(BIP 324) in the existing functional test framework and adds functional tests for the same. ### commits overview 1. introduces a new class `EncryptedP2PState` to store the keys, functions for performing the initial v2 handshake and encryption/decryption. 3. this class is used by `P2PConnection` in inbound/outbound connections to perform the initial v2 handshake before the v1 version handshake. Only after the initial v2 handshake is performed do application layer P2P messages(version, verack etc..) get exchanged. (in a v2 connection) - `v2_state` is the object of class `EncryptedP2PState` in `P2PConnection` used to store its keys, session-id etc. - a node [advertising](https://github.com/stratospher/blogosphere/blob/main/integration_test_bip324.md#advertising-to-support-v2-p2p) support for v2 P2P is different from a node actually [supporting v2 P2P](https://github.com/stratospher/blogosphere/blob/main/integration_test_bip324.md#supporting-v2-p2p) (differ when false advertisement of services occur) - introduce a boolean variable `supports_v2_p2p` in `P2PConnection` to denote if it supports v2 P2P. - introduce a boolean variable `advertises_v2_p2p` to denote whether `P2PConnection` which mimics peer behaviour advertises V2 P2P support. Default option is `False`. - In the test framework, you can create Inbound and Outbound connections to `TestNode` 1. During **Inbound Connections**, `P2PConnection` is the initiator [`TestNode` <--------- `P2PConnection`] - Case 1: - if the `TestNode` advertises/signals v2 P2P support (means `self.nodes[i]` set up with `"-v2transport=1"`), different behaviour will be exhibited based on whether: 1. `P2PConnection` supports v2 P2P 2. `P2PConnection` does not support v2 P2P - In a real world scenario, the initiator node would intrinsically know if they support v2 P2P based on whatever code they choose to run. However, in the test scenario where we mimic peer behaviour, we have no way of knowing if `P2PConnection` should support v2 P2P or not. So `supports_v2_p2p` boolean variable is used as an option to enable support for v2 P2P in `P2PConnection`. - Since the `TestNode` advertises v2 P2P support (using "-v2transport=1"), our initiator `P2PConnection` would send: 1. (if the `P2PConnection` supports v2 P2P) ellswift + garbage bytes to initiate the connection 2. (if the `P2PConnection` does not support v2 P2P) version message to initiate the connection - Case 2: - if the `TestNode` doesn't signal v2 P2P support; `P2PConnection` being the initiator would send version message to initiate a connection. 2. During **Outbound Connections** [TestNode --------> P2PConnection] - initiator `TestNode` would send: - (if the `P2PConnection` advertises v2 P2P) ellswift + garbage bytes to initiate the connection - (if the `P2PConnection` advertises v2 P2P) version message to initiate the connection - Suppose `P2PConnection` advertises v2 P2P support when it actually doesn't support v2 P2P (false advertisement scenario) - `TestNode` sends ellswift + garbage bytes - `P2PConnection` receives but can't process it and disconnects. - `TestNode` then tries using v1 P2P and sends version message - `P2PConnection` receives/processes this successfully and they communicate on v1 P2P 4. the encrypted P2P messages follow a different format - 3 byte length + 1-13 byte message_type + payload + 16 byte MAC 5. includes support for testing decoy messages and v2 connection downgrade(using false advertisement - when a v2 node makes an outbound connection to a node which doesn't support v2 but is advertised as v2 by some malicious intermediary) ### run the tests * functional test - `test/functional/p2p_v2_encrypted.py` `test/functional/p2p_v2_earlykeyresponse.py` I'm also super grateful to @ dhruv for his really valuable feedback on this branch. Also written a more elaborate explanation here - https://github.com/stratospher/blogosphere/blob/main/integration_test_bip324.md ACKs for top commit: naumenkogs: ACK bc9283c4415a932ec1eeb70ca2aa4399c80437b3 mzumsande: Code Review ACK bc9283c4415a932ec1eeb70ca2aa4399c80437b3 theStack: Code-review ACK bc9283c4415a932ec1eeb70ca2aa4399c80437b3 glozow: ACK bc9283c4415a932ec1eeb70ca2aa4399c80437b3 Tree-SHA512: 9b54ed27e925e1775e0e0d35e959cdbf2a9a1aab7bcf5d027e66f8b59780bdd0458a7a4311ddc7dd67657a4a2a2cd5034ead75524420d58a83f642a8304c9811
2024-01-26[rpc] return full string for package_msg and package-errorglozow
2024-01-23Merge bitcoin/bitcoin#28560: wallet, rpc: `FundTransaction` refactorAva Chow
18ad1b9142e91cef2f5c6a693eeb2d0fbb8c517d refactor: pass CRecipient to FundTransaction (josibake) 5ad19668dbcc47486d1c18f711cea3d8a9d2e7e2 refactor: simplify `CreateRecipients` (josibake) 47353a608dc6e20e5fd2ca53850d6f9aa3240d4a refactor: remove out param from `ParseRecipients` (josibake) f7384b921c3460c7a3cc7827a68b2c613bd98f8e refactor: move parsing to new function (josibake) 6f569ac903e5ddaac275996a5d0c31b2220b7b81 refactor: move normalization to new function (josibake) 435fe5cd96599c518e26efe444c9d94d1277996b test: add tests for fundrawtx and sendmany rpcs (josibake) Pull request description: ## Motivation The primary motivation for this PR is to enable `FundTransaction` to take a vector of `CRecipient` objects to allow passing BIP352 silent payment addresses to RPCs that use `FundTransaction` (e.g. `send`, `walletcreatefundedpsbt`). To do that, SFFO logic needs to be moved out of `FundTransaction` so the `CRecipient` objects with the correct SFFO information can be created and then passed to `FundTransaction`. As a secondary motivation, this PR moves the SFFO stuff closer to the caller, making the code cleaner and easier to understand. This is done by having a single function which parses RPC inputs for SFFO and consistently using the `set<int>` method for communicating SFFO. I'm also not convinced we need to pass a full `CMutableTx` object to `FundTransaction`, but I'm leaving that for a follow-up PR/discussion, as its not a blocker for silent payments. ACKs for top commit: S3RK: reACK 18ad1b9142e91cef2f5c6a693eeb2d0fbb8c517d josibake: > According to my `range-diff` nothing changed. reACK [18ad1b9](https://github.com/bitcoin/bitcoin/commit/18ad1b9142e91cef2f5c6a693eeb2d0fbb8c517d) achow101: ACK 18ad1b9142e91cef2f5c6a693eeb2d0fbb8c517d Tree-SHA512: d61f017cf7d98489ef216475b68693fd77e7b53a26a6477dcd73e7e5ceff5036b2d21476e377839e710bb73644759d42c4f9f4b14ed96b3e56ed87b07aa6d1a7
2024-01-23[rpc/net] Allow v2 p2p support in addconnectionstratospher
This test-only RPC is required when a TestNode initiates an outbound v2 p2p connection. Add a new arg `v2transport` so that the node can attempt v2 connections.
2024-01-22Merge bitcoin/bitcoin#29260: refactor: remove CTxMemPool::queryHashes()glozow
282b12ddb0aeb1f0991dd9f45c6b5c5c079652ec refactor: remove CTxMemPool::queryHashes() (stickies-v) Pull request description: `CTxMemPool::queryHashes()` is only used in `MempoolToJSON()`, where it can just as easily be replaced with the more general `CTxMemPool::entryAll()`. No behaviour change, just cleans up the code. ACKs for top commit: dergoegge: Code review ACK 282b12ddb0aeb1f0991dd9f45c6b5c5c079652ec TheCharlatan: ACK 282b12ddb0aeb1f0991dd9f45c6b5c5c079652ec glozow: ACK 282b12ddb0aeb1f0991dd9f45c6b5c5c079652ec. Looks like there's no conflicts. Tree-SHA512: 16160dec8e1f2457fa0f62dc96d2d2efd92c4bab810ecdb0e08918b8e85a667702c8e41421eeb4ea6abe92a5956a2a39a7a6368514973b78be0d22de2ad299b2
2024-01-19refactor: move parsing to new functionjosibake
Move the parsing and validation out of `AddOutputs` into its own function, `ParseOutputs`. This allows us to re-use this logic in `ParseRecipients` in a later commit, where the code is currently duplicated. The new `ParseOutputs` function returns a CTxDestination,CAmount tuples. This allows the caller to then translate the validated outputs into either CRecipients or CTxOuts.
2024-01-19refactor: move normalization to new functionjosibake
Move the univalue formatting logic out of AddOutputs and into its own function, `NormalizeOutputs`. This allows us to re-use this logic in later commits.
2024-01-18refactor: remove CTxMemPool::queryHashes()stickies-v
Its only usage can easily be replaced with CTxMemPool::entryAll()
2024-01-17rpc: Fix race in loadtxoutsetMarcoFalke
The tip may have advanced, also if it did not, there is no reason to have two variables point to the same block.
2024-01-12Make v2transport default for addnode RPC when enabledPieter Wuille
2024-01-12Merge bitcoin/bitcoin#28885: mempool / rpc: followup to ↵glozow
getprioritisedtransactions and delete a mapDeltas entry when delta==0 0eebd6fe7d01ddc7f6b7f13a6ed6e705c7aeae4e test: Assert that a new tx with a delta of 0 is never added (kevkevin) cfdbcd19b32fd63954d7947dcc639aef291fb6b2 rpc: exposing modified_fee in getprioritisedtransactions (kevkevin) 252a86729a15e47ed168d8da7c4a8d6113673909 rpc: renaming txid -> transactionid (kevkevin) 2fca6c2dd03c3955d86efb0b8d2a7961e42115fd rpc: changed prioritisation-map -> "" (kevkevin) 3a118e19e100110300d3290d4c1434f963721d94 test: Directly constructing 2 entry map for getprioritisedtransactions (kevkevin) Pull request description: In this PR I am addressing some comments in https://github.com/bitcoin/bitcoin/pull/27501 as a followup. - changed `prioritisation-map` in the `RPCResult` to `""` - Directly constructing 2 entry map for getprioritisedtransactions in functional tests - renamed `txid` to `transactionid` in `RPCResult` to be more consistent with naming elsewhere - exposed the `modified_fee` field instead of having it be a useless arg - Created a new test that asserts when `prioritisedtransaction` is called with a fee_delta of 0 it is not added to mempool ACKs for top commit: glozow: reACK 0eebd6fe7d01ddc7f6b7f13a6ed6e705c7aeae4e, only change is the doc suggestion Tree-SHA512: e99056e37a8b1cfc511d87c83edba7c928b50d2cd6c2fd7c038976779850677ad37fddeb2b983e8bc007ca8567eb21ebb78d7eae9b773657c2b297299993ec05
2024-01-11rpc: exposing modified_fee in getprioritisedtransactionskevkevin
Instead of having modified_fee be hidden we are now exposing it to avoid having useless code
2024-01-08rpc: renaming txid -> transactionidkevkevin
renamed to transactionid because it is named this way in getrawmempool and getmempoolancestors
2024-01-08rpc: changed prioritisation-map -> ""kevkevin
prioritisation-map gets eaten by the help generator to be "" so we are setting to "" to begin with
2024-01-08Merge bitcoin/bitcoin#29184: RPC/Blockchain: scanblocks: Accept named param ↵glozow
for filter_false_positives 5779010ed7be1cbe9b98a91c7487d3d14b7cf24d RPC/Blockchain: scanblocks: Accept named param for filter_false_positives (Luke Dashjr) Pull request description: Possibly due to a silent cross-merge, `scanblocks` was left out of 96233146dd31c1d99fd1619be4449944623ef750 ACKs for top commit: stickies-v: ACK 5779010ed7be1cbe9b98a91c7487d3d14b7cf24d theStack: ACK 5779010ed7be1cbe9b98a91c7487d3d14b7cf24d Tree-SHA512: bade107c7cb5fdd1265224c263a1e1edfc8bc0698b3abfac8d65c49a270181f0311713f7243813de17932a7a7ca65a36850e527ab0b433cf64c32191d3adde70
2024-01-05Remove GetAdjustedTimedergoegge
2024-01-05Merge bitcoin/bitcoin#28890: rpc: Remove deprecated -rpcserialversionfanquake
fa46cc22bc696e6845915ae91d6b68e36bf4c242 Remove deprecated -rpcserialversion (MarcoFalke) Pull request description: The flag is problematic for many reasons: * It is deprecated * It is a global flag, requiring a restart to change, as opposed to a flag that can be set on each RPC invocation * It may be hidden in config files by accident, hard to debug, causing LND crashes and bugs, see https://github.com/bitcoin/bitcoin/issues/28730#issuecomment-1780940868 * It makes performance improvements harder to implement: https://github.com/bitcoin/bitcoin/pull/17529#issuecomment-556082818 Fix all issues by removing it. If there is a use-case, likely a per-RPC flag can be added, if needed. ACKs for top commit: ajtowns: crACK fa46cc22bc696e6845915ae91d6b68e36bf4c242 TheCharlatan: lgtm ACK fa46cc22bc696e6845915ae91d6b68e36bf4c242 Tree-SHA512: 96ba1c60356ce93954fe5c2a59045771c6d1516ad0d9dc436ef1800a1f1b0153f0d5fb78ca99d53ad54ba25fbce36962bdf1d4325aceedfc8154a61347a6a915
2024-01-04RPC/Blockchain: scanblocks: Accept named param for filter_false_positivesLuke Dashjr
2023-12-14Merge bitcoin/bitcoin#29040: refactor: Remove pre-C++20 code, fs::path cleanupAva Chow
66667130416b86208e01a0eb5541a15ea805ac26 refactor: Rename fs::path::u8string() to fs::path::utf8string() (MarcoFalke) 856c88776f8486446602476a1c9e133ac0cff510 ArgsManager: return path by value from GetBlocksDirPath() (Vasil Dimov) fa3d9304e80c214c8b073f12a7f4b08c5a94af04 refactor: Remove pre-C++20 fs code (MarcoFalke) fa00098e1a493aa3cce20335d18e7f5f2fb7a4a8 Add tests for C++20 std::u8string (MarcoFalke) fa2bac08c22182e738a8cabf1b24a9dbf3b092d2 refactor: Avoid copy/move in fs.h (MarcoFalke) faea30227ba633da5ab257d0247853e0927244bb refactor: Use C++20 std::chrono::days (MarcoFalke) Pull request description: This: * Removes dead code. * Avoids unused copies in some places. * Adds copies in other places for safety. ACKs for top commit: achow101: ACK 66667130416b86208e01a0eb5541a15ea805ac26 ryanofsky: Code review ACK 66667130416b86208e01a0eb5541a15ea805ac26. Just documentation change since last review. stickies-v: re-ACK 66667130416b86208e01a0eb5541a15ea805ac26 Tree-SHA512: 6176e44f30b310d51632ec2d3827c3819905d0ddc6a4b57acfcb6cfa1f9735176da75ee8ed4a4abd1296cb0b83bee9374cc6f91ffac87c19b63c435eeadf3f46
2023-12-14Merge bitcoin/bitcoin#28051: Get rid of shutdown.cpp/shutdown.h, use ↵Ava Chow
SignalInterrupt directly 6db04be102807ee0120981a9b8de62a55439dabb Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly (Ryan Ofsky) 213542b625a6a4885fcbdfe236629a5f381eeb05 refactor: Add InitContext function to initialize NodeContext with global pointers (Ryan Ofsky) feeb7b816affa790e02e7ba0780c4ef33d2310ff refactor: Remove calls to StartShutdown from KernelNotifications (Ryan Ofsky) 6824eecaf1e74624cf149ed20abd9145c49d614a refactor: Remove call to StartShutdown from stop RPC (Ryan Ofsky) 1d92d89edbb1812dc353084c62772ebb1024d632 util: Get rid of uncaught exceptions thrown by SignalInterrupt class (Ryan Ofsky) ba93966368d3aaa426b97837ef475ec5aa612f5f refactor: Remove call to ShutdownRequested from IndexWaitSynced (Ryan Ofsky) 42e5829d9710ebebda5de356fab01dd7c149d5fa refactor: Remove call to ShutdownRequested from HTTPRequest (Ryan Ofsky) 73133c36aa9cc09546eabac18d0ea35274dd5d72 refactor: Add NodeContext::shutdown member (Ryan Ofsky) f4a8bd6e2f03e786a84dd7763d1c04665e6371f2 refactor: Remove call to StartShutdown from qt (Ryan Ofsky) f0c73c1336bee74fe2d58474ac36bca28c219e85 refactor: Remove call to ShutdownRequested from rpc/mining (Ryan Ofsky) 263b23f0082c60516acced1b03abb8e4d8f9ee46 refactor: Remove call to ShutdownRequested from chainstate init (Ryan Ofsky) Pull request description: This change drops `shutdown.h` and `shutdown.cpp` files, replacing them with a `NodeContext::shutdown` member which is used to trigger shutdowns directly. This gets rid of an unnecessary layer of indirection, and allows getting rid of the `kernel::g_context` global. Additionally, this PR tries to improve error handling of `SignalInterrupt` code by marking relevant methods `[[nodiscard]]` to avoid the possibility of uncaught exceptions mentioned https://github.com/bitcoin/bitcoin/pull/27861#discussion_r1255496707. Behavior is changing In a few cases which are noted in individual commit messages. Particularly: GUI code more consistently interrupts RPCs when it is shutting down, shutdown state no longer persists between unit tests, the stop RPC now returns an RPC error if requesting shutdown fails instead of aborting, and other failed shutdown calls now log errors instead of aborting. This PR is a net reduction in lines of code, but in some cases the explicit error handling and lack of global shutdown functions do make it more verbose. The verbosity can be seen as good thing if it discourages more code from directly triggering shutdowns, and instead encourages code to return errors or send notifications that could be translated into shutdowns. Probably a number of existing shutdown calls could just be replaced by better error handling. ACKs for top commit: achow101: ACK 6db04be102807ee0120981a9b8de62a55439dabb TheCharlatan: Re-ACK 6db04be102807ee0120981a9b8de62a55439dabb maflcko: ACK 6db04be102807ee0120981a9b8de62a55439dabb 👗 stickies-v: re-ACK 6db04be102807ee0120981a9b8de62a55439dabb Tree-SHA512: 7a34cb69085f37e813c43bdaded1a0cbf6c53bd95fdde96f0cb45346127fc934604c43bccd3328231ca2f1faf712a7418d047ceabd22ef2dca3c32ebb659e634
2023-12-14refactor: Rename fs::path::u8string() to fs::path::utf8string()MarcoFalke
2023-12-11Remove deprecated -rpcserialversionMarcoFalke
2023-12-07refactor: rpc: Pass CBlockIndex by reference instead of pointerMarcoFalke
All functions assume that the pointer is never null, so pass by reference, to avoid accidental segfaults at runtime, or at least make them more obvious. Also, remove unused c-style casts in touched lines. Also, add CHECK_NONFATAL checks, to turn segfault crashes into an recoverable runtime error with debug information.
2023-12-07refactor: Use reference instead of pointer in IsBlockPrunedMarcoFalke
This makes it harder to pass nullptr and cause issues such as https://github.com/bitcoin/bitcoin/commit/dde7ac5c704688c8a9af29bd07e5ae8114824ce7
2023-12-05rpc: fix getrawtransaction segfaultMartin Zumsande
The crash would happen when querying a mempool transaction with verbosity=2, while pruning.
2023-12-04refactor: Remove call to StartShutdown from stop RPCRyan Ofsky
Use SignalInterrupt object instead. There is a slight change in behavior here because the previous StartShutdown code used to abort on failure and the new code returns an RPC error instead.
2023-12-04refactor: Remove call to ShutdownRequested from rpc/miningRyan Ofsky
Use chainman.m_interrupt object instead There is no change in behavior in this commit
2023-12-01Merge bitcoin/bitcoin#28368: Fee Estimator updates from Validation ↵Andrew Chow
Interface/CScheduler thread 91504cbe0de2b74ef1aa2709761aaf0597ec66a2 rpc: `SyncWithValidationInterfaceQueue` on fee estimation RPC's (ismaelsadeeq) 714523918ba2b853fc69bee6b04a33ba0c828bf5 tx fees, policy: CBlockPolicyEstimator update from `CValidationInterface` notifications (ismaelsadeeq) dff5ad3b9944cbb56126ba37a8da180d1327ba39 CValidationInterface: modify the parameter of `TransactionAddedToMempool` (ismaelsadeeq) 91532bd38223d7d04166e05de11d0d0b55e60f13 tx fees, policy: update `CBlockPolicyEstimator::processBlock` parameter (ismaelsadeeq) bfcd401368fc0dc43827a8969a37b7e038d5ca79 CValidationInterface, mempool: add new callback to `CValidationInterface` (ismaelsadeeq) 0889e07987294d4ef2814abfca16d8e2a0c5f541 tx fees, policy: cast with static_cast instead of C-Style cast (ismaelsadeeq) a0e3eb7549d2ba4dd3af12b9ce65e29158f59078 tx fees, policy: bugfix: move `removeTx` into reason != `BLOCK` condition (ismaelsadeeq) Pull request description: This is an attempt to #11775 This Pr will enable fee estimator to listen to ValidationInterface notifications to process new transactions added and removed from the mempool. This PR includes the following changes: - Added a new callback to the Validation Interface `MempoolTransactionsRemovedForConnectedBlock`, which notifies listeners about the transactions that have been removed due to a new block being connected, along with the height at which the transactions were removed. - Modified the `TransactionAddedToMempool` callback parameter to include additional information about the transaction needed for fee estimation. - Updated `CBlockPolicyEstimator` to process transactions using` CTransactionRef` instead of `CTxMempoolEntry.` - Implemented the `CValidationInterface` interface in `CBlockPolicyEstimater` and overridden the `TransactionAddedToMempool`, `TransactionRemovedFromMempool`, and `MempoolTransactionsRemovedForConnectedBlock` methods to receive updates from their notifications. Prior to this PR, the fee estimator updates from the mempool, i.e whenever a new block is connected all transactions in the block that are in our mempool are going to be removed using the `removeForBlock` function in `txmempool.cpp`. This removal triggered updates to the fee estimator. As a result, the fee estimator would block mempool's `cs` until it finished updating every time a new block was connected. Instead of being blocked only on mempool tx removal, we were blocking on both tx removal and fee estimator updating. If we want to further improve fee estimation, or add heavy-calulation steps to it, it is currently not viable as we would be slowing down block relay in the process This PR is smaller in terms of the changes made compared to #11775, as it focuses solely on enabling fee estimator updates from the validationInterface/cscheduler thread notifications. I have not split the validation interface because, as I understand it, the rationale behind the split in #11775 was to have `MempoolInterface` signals come from the mempool and `CValidationInterface` events come from validation. I believe this separation can be achieved in a separate refactoring PR when the need arises. Also left out some commits from #11775 - Some refactoring which are no longer needed. - Handle reorgs much better in fee estimator. - Track witness hash malleation in fee estimator I believe they are a separate change that can come in a follow-up after this. ACKs for top commit: achow101: ACK 91504cbe0de2b74ef1aa2709761aaf0597ec66a2 TheCharlatan: Re-ACK 91504cbe0de2b74ef1aa2709761aaf0597ec66a2 willcl-ark: ACK 91504cbe0de2b74ef1aa2709761aaf0597ec66a2 Tree-SHA512: 846dfb9da57a8a42458827b8975722d153907fe6302ad65748d74f311e1925557ad951c3d95fe71fb90ddcc8a3710c45abb343ab86b88780871cb9c38c72c7b1
2023-12-01Merge bitcoin/bitcoin#28784: rpc: keep `.cookie` file if it was not generatedAndrew Chow
7cb9367157eb42ee06bc6fa024522cc14a80138d rpc: keep .cookie if it was not generated (Roman Zeyde) Pull request description: Otherwise, starting bitcoind twice may cause the `.cookie` file generated by the first instance to be deleted by the second instance shutdown (after failing to obtain a lock). ACKs for top commit: willcl-ark: re-ACK 7cb9367157eb42ee06bc6fa024522cc14a80138d achow101: ACK 7cb9367157eb42ee06bc6fa024522cc14a80138d kristapsk: re-ACK 7cb9367157eb42ee06bc6fa024522cc14a80138d stickies-v: ACK 7cb9367157eb42ee06bc6fa024522cc14a80138d Tree-SHA512: 0960dbc457975b0e0535f3d814824a879d7f85c9f1191537415b3fc253429a316a8e4badde56c8bc139778f132392983cec5fbe03891fb15ff61d3bc3f6e681b
2023-12-01Merge bitcoin/bitcoin#28848: bugfix, Change up submitpackage results to ↵Andrew Chow
return results for all transactions f23ba24aa079d68697d475789cd21bd7b5075550 test_submitpackage: only make a chain of 3 txns (Greg Sanders) e67a345162912ef7c1bfa3c89c7e7c629505f0a3 doc: submitpackage vsize results are sigops-adjusted (Greg Sanders) b67db52c399089e5d4c4202ebb905794dfd050d0 RPC submitpackage: change return format to allow partial errors (Greg Sanders) Pull request description: This was prompted by errors being returned that didn't "make any sense" to me, because it would for example return a "fee too low" error, when the "real" error was the child had something invalid, which disallowed CPFP evaluation. Rather than make judgment calls on what error is important(which is currently just return the "first"!), we simply return all errors and let the callers determine what's best. Added a top level `package_msg` for quick eye-balling of general success of the package. This PR also fixes a couple bugs: 1) Currently we don't actually broadcast a transaction, even if it was entered into our mempool, if a subsequent transaction causes `PKG_TX` failure. 2) "other-wtxid" is uncovered by tests, but IIUC was previously required to return "fees" and "vsize" results, but did not. I just make those results optional. ACKs for top commit: Sjors: Light re-utACK f23ba24aa079d68697d475789cd21bd7b5075550 achow101: ACK f23ba24aa079d68697d475789cd21bd7b5075550 glozow: utACK f23ba24aa079d68697d475789cd21bd7b5075550, thanks for taking the suggestions Tree-SHA512: ebfd716a4fed9e8c2dea3d2181ba6a6171b06718d29ac2324c67b7a30b374d199f7e1739f91ab5d036be172d0479de9bc89c32263ee62143c0338b9b622d0cca
2023-11-30Rename version.h to node/protocol_version.hMarcoFalke
generated by cgit v1.2.3 (git 2.26.2) at 2024-10-13 03:29:24 +0000