22531 Commits

Author SHA1 Message Date
josibake
0760ce0b9e
test: add missing BOOST_ASSERT
this was missed in the original PR
2022-08-10 15:19:32 +02:00
josibake
db09aec937
wallet: switch to new shuffle, erase, push_back
switch to new methods, remove old code. this also
updates the Size, All, and Clear methods to now use
the coins map.

this commit is not strictly a refactor because previously
coin selection was never run over the UNKNOWN type until the last
step when being run over all. now that we are iterating over each,
it is run over UNKNOWN but this is expected to be empty most of the time.

Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
2022-08-10 15:19:31 +02:00
josibake
b6b50b0f2b
scripted-diff: Uppercase function names
Change `CoinsResult` functions to uppercase to be consistent with
the style guide.

-BEGIN VERIFY SCRIPT-
git grep -l "available_coins" | grep -v mempool_stress.cpp | xargs sed -i "s/available_coins\.\(size\|all\|clear\)/available_coins\.\u\1/"
git grep -l AvailableCoins | xargs sed -i "/AvailableCoins/ s/\(all()\|size()\|clear()\)/\u\1/"
sed -i "s/\(clear()\|all()\|size()\)/\u&/g" src/wallet/spend.h
sed -i "/CoinsResult::/ s/\(clear()\|all()\|size()\)/\u&/" src/wallet/spend.cpp
sed -i "s/result.size/result.Size/" src/wallet/spend.cpp
sed -i "s/this->size/this->Size/" src/wallet/spend.cpp
-END VERIFY SCRIPT-
2022-08-10 15:19:31 +02:00
josibake
3f27a2adce
refactor: add new helper methods
add Shuffle, Erase, and Add to CoinsResult struct
add a helper function for mapping TxoutType to OutputType

Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
2022-08-10 15:19:18 +02:00
josibake
f5649db9d5
refactor: add UNKNOWN OutputType
add to enum, array and handle UNKNOWN in various case statements
2022-08-10 10:17:54 +02:00
Andrew Chow
ac59112a6a
Merge bitcoin/bitcoin#23480: Add rawtr() descriptor for P2TR with specified (tweaked) output key
544b4332f0e122167bdb94dc963405422faa30cb Add wallet tests for spending rawtr() (Pieter Wuille)
e1e3081200a71b6c9b0dcf236bc2a37ed1aa7552 If P2TR tweaked key is available, sign with it (Pieter Wuille)
8d9670ccb756592bddb2a269bf5078d62658537b Add rawtr() descriptor for P2TR with unknown tweak (Pieter Wuille)

Pull request description:

  It may be useful to be able to represent P2TR outputs in descriptors whose script tree and/or internal key aren't known. This PR does that, by adding a `rawtr(KEY)` descriptor, where the KEY represents the output key directly. If the private key corresponding to that output key is known, it also permits signing with it.

  I'm not convinced this is desirable, but presumably "tr(KEY)" sounds more intended for direct use than "rawtr(KEY)".

ACKs for top commit:
  achow101:
    ACK 544b4332f0e122167bdb94dc963405422faa30cb
  sanket1729:
    code review ACK 544b4332f0e122167bdb94dc963405422faa30cb
  w0xlt:
    reACK 544b4332f0

Tree-SHA512: 0de08de517468bc22ab0c00db471ce33144f5dc211ebc2974c6ea95709f44e830532ec5cdb0128c572513d352120bd651c4559516d4500b5b0a3d257c4b45aca
2022-08-09 16:36:00 -04:00
glozow
c012875b9d
Merge bitcoin/bitcoin#24564: doc: Clarify that CheckSequenceLocksAtTip is a validation function
fa8671018766b2f0e18c94cff3ab2a67c6b3a41d Clarify that CheckSequenceLocksAtTip is a validation function (MarcoFalke)

Pull request description:

  It has been pointed out that a bug in this function can prevent block template creation. ( https://github.com/bitcoin/bitcoin/pull/24080#issuecomment-1065148776 ) So it seems that the scope of this function is more than "policy". Rename it back to "validation", to partially revert commit fa4e30b0f36f2e7a09db7d30dca9008ed9dbcb35.

ACKs for top commit:
  ajtowns:
    ACK fa8671018766b2f0e18c94cff3ab2a67c6b3a41d - looks fine to me
  glozow:
    ACK fa8671018766b2f0e18c94cff3ab2a67c6b3a41d

Tree-SHA512: 2e0df8c70df4cbea857977f140a8616cfa7505e74df66c9c9fbcf184670ce3ce7567183c3f76e6f3fe8ca6de0e065b9babde6352d6cb495e71ea077ddedbc3f4
2022-08-09 11:51:55 +01:00
fanquake
9ff6adc43a
Merge bitcoin/bitcoin#25804: Update translations for 24.0 string freeze
ff52b24e5c9b3dca269e66e0d78051a2aa5f1462 qt: Update translation source file (Hennadii Stepanov)
15f762fc65af5bd34f4ea52f2b43bcc86dcea80a qt: Bump Transifex slug for 24.x (Hennadii Stepanov)

Pull request description:

  Required to open Transifex translations for 24.0 (see bitcoin/bitcoin#24987).

ACKs for top commit:
  laanwj:
    Changes-match-release-process ACK ff52b24e5c9b3dca269e66e0d78051a2aa5f1462
  jarolrod:
    ACK ff52b24e5c9b3dca269e66e0d78051a2aa5f1462

Tree-SHA512: f3e65b1608818084f4a3adddd2a58541ebe91ebcdb3717da2eb6f4147a0fc5f0d536a2e9f8b4eacc2a580b12c619d9eec391bfdcc5e81fa02f527408ec73a984
2022-08-08 16:28:07 +01:00
Andrew Chow
a478c5350a
Merge bitcoin/bitcoin#25790: wallet: improve {LoadActive,Deactivate}ScriptPubKeyMan log
b5a762a35368ad5ab07018e5da14229291a54b94 wallet: improve `{LoadActive,Deactivate}ScriptPubKeyMan` log (w0xlt)

Pull request description:

  This PR includes the output type description in the log. It currently shows the enum position, which is only useful if the reader knows the code.

  Master:
  ```
  Setting spkMan to active: id = 9f..04, type = 3, internal = 0
  Setting spkMan to active: id = 3d..21, type = 2, internal = 0
  Setting spkMan to active: id = 69..d4, type = 0, internal = 1
  Setting spkMan to active: id = 97..ea, type = 1, internal = 1
  ```

  PR:
  ```
  Setting spkMan to active: id = 6a..4f, type = bech32m, internal = false
  Setting spkMan to active: id = 83..dc, type = legacy, internal = true
  Setting spkMan to active: id = 7e..5d, type = p2sh-segwit, internal = true
  Setting spkMan to active: id = bd..d2, type = bech32, internal = true
  Setting spkMan to active: id = 13...7c, type = bech32m, internal = true

  ```

ACKs for top commit:
  S3RK:
    Code review ACK b5a762a35368ad5ab07018e5da14229291a54b94
  achow101:
    ACK b5a762a35368ad5ab07018e5da14229291a54b94
  theStack:
    Code-review ACK b5a762a35368ad5ab07018e5da14229291a54b94

Tree-SHA512: 5a79706d5452e523b0456fb8435545c6c8e550b6722c0d7966af79011275a97ed97cab297562e031d601aa855118082c5b770af118783b1faaaec0cba9f9ee6a
2022-08-08 11:18:08 -04:00
Hennadii Stepanov
ff52b24e5c
qt: Update translation source file 2022-08-08 12:18:55 +01:00
w0xlt
b5a762a353 wallet: improve {LoadActive,Deactivate}ScriptPubKeyMan log 2022-08-05 17:19:52 -03:00
Andrew Chow
59bd6b6d37
Merge bitcoin/bitcoin#24699: wallet: Improve AvailableCoins performance by reducing duplicated operations
bc886fcb31e1afa7bbf7b86bfd93e51da7076ccf Change mapWallet to be a std::unordered_map (Andrew Chow)
272356024db978c92112167f8d8e4cc62adad63d Change getWalletTxs to return a set instead of a vector (Andrew Chow)
97532867cf51db3e941231fbdc60f9f4fa0012a0 Change mapTxSpends to be a std::unordered_multimap (Andrew Chow)
1f798fe85ba952273005f68e36ed48cfc36f4c9d wallet: Cache SigningProviders (Andrew Chow)
8a105ecd1aeff15f84c3883e2762bf71ad59d920 wallet: Use CalculateMaximumSignedInputSize to indicate solvability (Andrew Chow)

Pull request description:

  While running my coin selection simulations, I noticed that towards the end of the simulation, the wallet would become slow to make new transactions. The wallet generally performs much more slowly when there are a large number of transactions and/or a large number of keys. The improvements here are focused on wallets with a large number of transactions as that is what the simulations produce.

  Most of the slowdown I observed was due to `DescriptorScriptPubKeyMan::GetSigningProvider` re-deriving keys every time it is called. To avoid this, it will now cache the `SigningProvider` produced so that repeatedly fetching the `SigningProvider` for the same script will not result in the same key being derived over and over. This has a side effect of making the function non-const, which makes a lot of other functions non-const as well. This helps with wallets with lots of address reuse (as my coin selection simulations are), but not if addresses are not reused as keys will end up needing to be derived the first time `GetSigningProvider` is called for a script.

  The `GetSigningProvider` problem was also exacerbated by unnecessarily fetching a `SigningProvider` for the same script multiple times. A `SigningProvider` is retrieved to be used inside of `IsSolvable`. A few lines later, we use `GetTxSpendSize` which fetches a `SigningProvider` and then calls `CalculateMaximumSignedInputSize`. We can avoid a second call to `GetSigningProvider` by using `CalculateMaximumSignedInputSize` directly with the `SigningProvider` already retrieved for `IsSolvable`.

  There is an additional slowdown where `ProduceSignature` with a dummy signer is called twice for each output. The first time is `IsSolvable` checks that `ProduceSignature` succeeds, thereby informing whether we have solving data. The second is `CalculateMaximumSignedInputSize` which returns -1 if `ProduceSignature` fails, and returns the input size otherwise. We can reduce this to one call of `ProduceSignature` by using `CalculateMaximumSignedInputSize`'s result to set `solvable`.

  Lastly, a lot of time is spent looking in `mapWallet` and `mapTxSpends` to determine whether an output is already spent. The performance of these lookups is slightly improved by changing those maps to use `std::unordered_map` and `std::unordered_multimap` respectively.

ACKs for top commit:
  Xekyo:
    ACK bc886fcb31e1afa7bbf7b86bfd93e51da7076ccf
  furszy:
    diff re-reACK bc886fcb

Tree-SHA512: fd710fe1224ef67d2bb83d6ac9e7428d9f76a67f14085915f9d80e1a492d2c51cb912edfcaad1db11c2edf8d2d97eb7ddd95bfb364587fb1f143490fd72c9ec1
2022-08-05 15:31:45 -04:00
Andrew Chow
35305c759a
Merge bitcoin/bitcoin#22751: rpc/wallet: add simulaterawtransaction RPC
db10cf8ae36693cb4d3ed1b47b84709cf9c0d849 rpc/wallet: add simulaterawtransaction RPC (Karl-Johan Alm)
701a64f548662e01821765b2934b6e4b321fda6d test: add support for Decimal to assert_approx (Karl-Johan Alm)

Pull request description:

  (note: this was originally titled "add analyzerawtransaction RPC")

  This command iterates over the inputs and outputs of the given transactions, and tallies up the balance change for the given wallet. This can be useful e.g. when verifying that a coin join like transaction doesn't contain unexpected inputs that the wallet will then sign for unintentionally.

  I originally proposed this to Elements (https://github.com/ElementsProject/elements/pull/1016) and it was suggested that I propose this upstream.

  There is an alternative #22776 to instead add this info to `getbalances` when providing an optional transaction as argument.

ACKs for top commit:
  jonatack:
    ACK db10cf8ae36693cb4d3ed1b47b84709cf9c0d849
  achow101:
    re-ACK db10cf8ae36693cb4d3ed1b47b84709cf9c0d849

Tree-SHA512: adf222ec7dcdc068d007ae6f465dbc35b692dc7bb2db337be25340ad0c2f9c64cfab4124df23400995c700f41c83c29a2c34812121782c26063b100c7969b89d
2022-08-05 15:19:03 -04:00
MacroFake
7d3817b29a
Merge bitcoin/bitcoin#25760: rest: clean-up for mempool endpoints
acbea66589100fe6ef726f4b2a92ec26132ef17b rest: clean-up for `mempool` endpoints (brunoerg)

Pull request description:

  The functions `rest_mempool_info` and `rest_mempool_contents` are similar, the only difference between them is:
  `rest_mempool_info` uses `MempoolInfoToJSON` to get the mempool informations and `rest_mempool_contents` uses `MempoolToJSON`, for this reason this PR creates a new function to handle it and reduce duplicated code.

  Also,
  1. Rename `strURIPart` to `str_uri_part`.
  2. Rename `strJSON` to `str_json`.

ACKs for top commit:
  stickies-v:
    re-ACK acbea66589100fe6ef726f4b2a92ec26132ef17b - verified that just the error message was updated since da0c612c3d
  theStack:
    re-ACK acbea66589100fe6ef726f4b2a92ec26132ef17b

Tree-SHA512: 35f6f0732a573fe8a6cdcc782f89ae3427a1de19f069a68c9c51bb525118c2b07e20303cbe19b9d4b7d1ad055d69c32def2d0fb8f886c851da562dd9ce33ad6a
2022-08-05 16:43:29 +02:00
MacroFake
006740b6f6
Merge bitcoin/bitcoin#25721: refactor: Replace BResult with util::Result
a23cca56c0a7f4a267915b4beba3af3454c51603 refactor: Replace BResult with util::Result (Ryan Ofsky)

Pull request description:

  Rename `BResult` class to `util::Result` and update the class interface to be more compatible with `std::optional` and with a full-featured result class implemented in https://github.com/bitcoin/bitcoin/pull/25665. Motivation for this change is to update existing `BResult` usages now so they don't have to change later when more features are added in https://github.com/bitcoin/bitcoin/pull/25665.

  This change makes the following improvements originally implemented in https://github.com/bitcoin/bitcoin/pull/25665:

  - More explicit API. Drops potentially misleading `BResult` constructor that treats any bilingual string argument as an error. Adds `util::Error` constructor so it is never ambiguous when a result is being assigned an error or non-error value.

  - Better type compatibility. Supports `util::Result<bilingual_str>` return values to hold translated messages which are not errors.

  - More standard and consistent API. `util::Result` supports most of the same operators and methods as `std::optional`. `BResult` had a less familiar interface with `HasRes`/`GetObj`/`ReleaseObj` methods. The Result/Res/Obj naming was also not internally consistent.

  - Better code organization. Puts `src/util/` code in the `util::` namespace so naming reflects code organization and it is obvious where the class is coming from. Drops "B" from name because it is undocumented what it stands for (bilingual?)

  - Has unit tests.

ACKs for top commit:
  MarcoFalke:
    ACK a23cca56c0a7f4a267915b4beba3af3454c51603 🏵
  jonatack:
    ACK a23cca56c0a7f4a267915b4beba3af3454c51603

Tree-SHA512: 2769791e08cd62f21d850aa13fa7afce4fb6875a9cedc39ad5025150dbc611c2ecfd7b3aba8b980a79fde7fbda13babdfa37340633c69b501b6e89727bad5b31
2022-08-05 15:33:45 +02:00
brunoerg
acbea66589 rest: clean-up for mempool endpoints 2022-08-05 10:28:11 -03:00
fanquake
e038605585
Merge bitcoin/bitcoin#24662: addrman: Use system time instead of adjusted network time
fadd8b2676f6d68ec87189871461c9a6a6aa3cac addrman: Use system time instead of adjusted network time (MarcoFalke)

Pull request description:

  This changes addrman to use system time for address relay instead of the network adjusted time.

  This is an improvement, because network time has multiple issues:

  * It is non-monotonic, even if the system time is monotonic.
  * It may be wrong, even if the system time is correct.
  * It may be wrong, if the system time is wrong. For example, when the node has limited number of connections (`4`), or the system time is wrong by too much (more than +-70 minutes), or the system time only got wrong after timedata collected more than half of the entries while the time was correct, ...)

  This may slightly degrade addr relay for nodes where timedata successfully adjusted the time. Addr relay can already deal with minor offsets of up to 10 minutes. Offsets larger than this should still allow addr relay and not result in a DoS.

ACKs for top commit:
  dergoegge:
    Code review ACK fadd8b2676f6d68ec87189871461c9a6a6aa3cac

Tree-SHA512: b6c178fa01161544e5bc76c4cb23e11bcc30391f7b7a64accce864923766647bcfce2e8ae21d36fb1ffc1afa07bc46415aca612405bd8d4cc1f319c92a08498f
2022-08-05 09:03:33 +01:00
Karl-Johan Alm
db10cf8ae3
rpc/wallet: add simulaterawtransaction RPC
This command iterates over the inputs and outputs of the given transactions, and tallies up the balance change for the given wallet. This can be useful e.g. when verifying that a coin join like transaction doesn't contain unexpected inputs that the wallet will then sign for unintentionally.
2022-08-05 09:48:09 +09:00
fanquake
e09ad284c7
Merge bitcoin/bitcoin#24675: util: Use ArgsManager::GetPathArg more widely
b01f336708019f8c8274ea701d3446e4123e7af2 util, refactor: Drop explicit conversion to fs::path (Hennadii Stepanov)
138c668e2b4d64279ddefbe07c1d9b7c3d3c537c util, refactor: Use GetPathArg to read "-rpccookiefile" value (Hennadii Stepanov)
1276090705060fcc97072481c2383bbaaa556194 util, refactor: Use GetPathArg to read "-conf" value (Hennadii Stepanov)

Pull request description:

  This PR is a continuation of bitcoin/bitcoin#24265 and bitcoin/bitcoin#24306.

  Now the following command-line arguments / configure options been read with the `GetPathArg` method:
  - `-conf`, also `includeconf` values been normalized
  - `-rpccookiefile`

ACKs for top commit:
  jarolrod:
    Code Review ACK b01f336708019f8c8274ea701d3446e4123e7af2
  ryanofsky:
    Code review ACK b01f336708019f8c8274ea701d3446e4123e7af. Changes since last review: just dropping first commit (NormalizedPathFromString) as suggested

Tree-SHA512: 2d26d50b73542acdbcc63a32068977b2a49a017d31ca337471a0446f964eb0a6e3e4e3bb1ebe6771566a260f2cae3bc2ebe93b4b523183cea0d51768daab85c9
2022-08-04 16:58:01 +01:00
fanquake
36c83b40bd
Merge bitcoin/bitcoin#25023: Remove unused SetTip(nullptr) code
faab8dceb37a944d0763fcca390342111e6a9fcc Remove unused SetTip(nullptr) code (MacroFake)

Pull request description:

  Now that this path is no longer used after commit b51e60f91472da5216116626afc032acd5616e85, we can remove it.

  Future code should reset `CChain` by simply discarding it and constructing a fresh one.

ACKs for top commit:
  ryanofsky:
    Code review ACK faab8dceb37a944d0763fcca390342111e6a9fcc. Just moved an assert statement since last review

Tree-SHA512: 7dc273b11133d85d32ca2a69c0c7c07b39cdd338141ef5b51496e7de334a809864d5459eb95535497866c8b1e468aae84ed8f91b543041e6ee20130d5622874e
2022-08-04 16:48:14 +01:00
Andrew Chow
bc886fcb31 Change mapWallet to be a std::unordered_map 2022-08-03 15:33:15 -04:00
Andrew Chow
272356024d Change getWalletTxs to return a set instead of a vector
For some reason, the primary consumer of getWalletTxs requires the
transactions to be in hash order when it is processing them. std::map
will iterate in hash order so the transactions end up in that order when
placed into the vector. To ensure this order when mapWallet is no longer
ordered, the vector is replaced with a set which will maintain the hash
order.
2022-08-03 15:33:15 -04:00
Andrew Chow
97532867cf Change mapTxSpends to be a std::unordered_multimap 2022-08-03 15:33:15 -04:00
Andrew Chow
1f798fe85b wallet: Cache SigningProviders
In order to avoid constantly re-deriving the same keys in
DescriptorScriptPubKeyMan, cache the SigningProviders generated inside
of GetSigningProvider.
2022-08-03 15:33:13 -04:00
Carl Dong
0f3a2532c3 validationcaches: Use size_t for sizes
...also move the 0-clamping logic to ApplyArgsManOptions, where it
   belongs.
2022-08-03 12:03:28 -04:00
Carl Dong
41c5201a90 validationcaches: Add and use ValidationCacheSizes
Also:

- Make DEFAULT_MAX_SIG_CACHE_SIZE into constexpr
  DEFAULT_MAX_SIG_CACHE_BYTES to utilize the compile-time integer
  arithmetic overflow checking available to constexpr.
- Fix comment (MiB instead of MB) for DEFAULT_MAX_SIG_CACHE_BYTES.
- Pass in max_size_bytes parameter to InitS*Cache(), modify log line to
  no longer allude to maxsigcachesize being split evenly between the two
  validation caches.
- Fix possible integer truncation and add a comment.

[META] I've kept the integer types as int64_t in order to not introduce
       unintended behaviour changes, in the next commit we will make
       them size_t.
2022-08-03 12:03:27 -04:00
Carl Dong
82d3058539 cuckoocache: Check for uint32 overflow in setup_bytes
This fixes an potential overflow which existed prior to this patchset.

If CuckooCache::cache<Element, Hash>::setup_bytes is called with a
`size_t bytes` which, when divided by sizeof(Element), does not fit into
an uint32_t, the implicit conversion to uint32_t in the call to setup
will result in an overflow.

At least on x86_64, this overflow is possible:

static_assert(std::numeric_limits<size_t>::max() / 32 <= std::numeric_limits<uint32_t>::max());
static_assert(std::numeric_limits<size_t>::max() / 4 <= std::numeric_limits<uint32_t>::max());

This commit detects such cases and signals to callers that the `size_t
bytes` input is too large.
2022-08-03 12:02:32 -04:00
Carl Dong
b370164b31 validationcaches: Abolish arbitrary limit
1. -maxsigcachesize is a DEBUG_ONLY option

2. Almost 7 years has passed since its semantics change in
   830e3f3d027ba5c8121eed0f6a9ce99961352572 from "number of entries" to
   "number of mebibytes"

3. A std::new_handler was added to the codebase after the original PR
   which introduced this limit, which will terminate immediately instead
   of causing trouble by being caught somewhere unexpected.
2022-08-03 12:02:31 -04:00
Carl Dong
08dbc6ef72 cuckoocache: Return approximate memory size
Returning the approximate total size eliminates the need for
InitS*Cache() to do nElems*sizeof(uint256). The cuckoocache has a better
idea of this information.
2022-08-03 12:02:31 -04:00
Carl Dong
0dbce4b103 tests: Reduce calls to InitS*Cache()
In src/test/fuzz/script_sigcache.cpp, we should really be setting up a
full working BasicTestingSetup. The initialize_ function is only run
once anyway.

In src/test/txvalidationcache_tests.cpp, the Dersig100Setup inherits
from BasicTestingSetup, which should have already set up a global script
execution cache without the need to explicitly call
InitScriptExecutionCache.
2022-08-03 12:02:31 -04:00
Ryan Ofsky
a23cca56c0 refactor: Replace BResult with util::Result
Rename `BResult` class to `util::Result` and update the class interface to be
more compatible with `std::optional` and with a full-featured result class
implemented in https://github.com/bitcoin/bitcoin/pull/25665. Motivation for
this change is to update existing `BResult` usages now so they don't have to
change later when more features are added in #25665.

This change makes the following improvements originally implemented in #25665:

- More explicit API. Drops potentially misleading `BResult` constructor that
  treats any bilingual string argument as an error. Adds `util::Error`
  constructor so it is never ambiguous when a result is being assigned an error
  or non-error value.

- Better type compatibility. Supports `util::Result<bilingual_str>` return
  values to hold translated messages which are not errors.

- More standard and consistent API. `util::Result` supports most of the same
  operators and methods as `std::optional`. `BResult` had a less familiar
  interface with `HasRes`/`GetObj`/`ReleaseObj` methods. The Result/Res/Obj
  naming was also not internally consistent.

- Better code organization. Puts `src/util/` code in the `util::` namespace so
  naming reflects code organization and it is obvious where the class is coming
  from. Drops "B" from name because it is undocumented what it stands for
  (bilingual?)

- Has unit tests.
2022-08-03 07:33:01 -04:00
MacroFake
fad5bc432b
test: Add missing static to IsStandardTx helper 2022-08-03 11:19:53 +02:00
glozow
f6fdedf850
Merge bitcoin/bitcoin#25648: refactor: Remove all policy globals
ddddd6913b1bdee1cad89a32d363306ea1f7b8d7 sort after scripted-diff (MacroFake)
fac812ca835e0d843aba1d4db0e49d183018a29e scripted-diff: Move mempool_args to src/node (MacroFake)
66664384a6fec39ecb4d8d06db66a4f193a06e33 Remove ::g_max_datacarrier_bytes global (MacroFake)
fad0b4fab849eb5f1f0aa54ebc290f85a473ec91 Pass datacarrier setting into IsStandard (MacroFake)
fa2a6b8516b24d7e9ca11926a49cf2b07f661e81 Combine datacarrier globals into one (MacroFake)
fa477d32eefcc3dd2f06b452066290d9936d8c5d Remove ::GetVirtualTransactionSize() alias (MacroFake)
fa2f6c1a611dffe5a3f63fe1b453f1dd420371b1 Remove ::fIsBareMultisigStd global (MacroFake)
fadc14e4f514e7167723285e0ac3d4a7149bbee6 Remove ::dustRelayFee (MacroFake)
fa8a7f01fe1b6db98097021276ed5d929faadbec Remove ::IsStandardTx(tx, reason) alias (MacroFake)
fa7a9114e59b81b50584311a4ab2b3e9a8d956bd test: Remove unused cs_main (MacroFake)
fa9cba7afb73c01bd2c8fefd662dfc80dd98c5e8 Remove ::incrementalRelayFee and ::minRelayTxFee globals (MacroFake)
fa148602e67fe035b1b21eff6c0b656919ac2d45 Remove ::fRequireStandard global (MacroFake)
fa468bdfb62dec286cb977db78d3e47b64dafeba Return optional error from ApplyArgsManOptions (MacroFake)

Pull request description:

  This change is good because:

  * It moves module-specific init-logic out of the bloated init.cpp
  * It removes a global from validation.cpp and places it into the data structure that needs it (mempool)

ACKs for top commit:
  glozow:
    re ACK ddddd69
  ryanofsky:
    Code review ACK ddddd6913b1bdee1cad89a32d363306ea1f7b8d7
  ariard:
    Light Code Review ACK ddddd69

Tree-SHA512: 9de2ce601cfcaa4dfd7d1c92270568895ce8702ccdffb59829fbe9618eab0fd88d738afef33ed66988c66861115e0340e881056bfb71e2aed4af2440bd37eb1e
2022-08-03 09:47:01 +01:00
MacroFake
faab8dceb3
Remove unused SetTip(nullptr) code 2022-08-03 09:21:53 +02:00
Andrew Chow
de3c46c938
Merge bitcoin/bitcoin#25272: wallet: guard and alert about a wallet invalid state during chain sync
9e04cfaa76cf9dda27f10359dd43e78dd3268e09 test: add coverage for wallet inconsistent state during sync (furszy)
77de5c693ffe8dc0afa5e40126e9b0e9cc547e04 wallet: guard and alert about a wallet invalid state during chain sync (furszy)

Pull request description:

  Follow-up work to my comment in #25239.

  Guarding and alerting the user about a wallet invalid state during chain synchronization.

  #### Explanation
  if the `AddToWallet` tx write fails, the method returns a wtx `nullptr` without removing the recently added transaction from the wallet's map.

  Which makes that `AddToWalletIfInvolvingMe` return false (even when the tx is on the wallet's map already), --> which makes `SyncTransaction` skip the `MarkInputsDirty` call --> which leads to a wallet invalid state where the inputs of this new transaction are not marked dirty, while the transaction that spends them still exist on the in-memory wallet tx map.

  Plus, as we only store the arriving transaction inside `AddToWalletIfInvolvingMe` when we synchronize/scan block/s from the chain and nowhere else, it makes sense to treat the transaction db write error as a runtime error to notify the user about the problem. Otherwise, the user will lose all the not stored transactions after a wallet shutdown (without be able to recover them automatically on the next startup because the chain sync would be above the block where the txs arrived).

  Note:
  On purpose, the first commit adds test coverage for it. Showing how the wallet can end up in an invalid state. The second commit corrects it with the proposed solution.

ACKs for top commit:
  achow101:
    re-ACK 9e04cfaa76cf9dda27f10359dd43e78dd3268e09
  jonatack:
    ACK 9e04cfaa76cf9dda27f10359dd43e78dd3268e09

Tree-SHA512: 81f765eca40547d7764833d8ccfae686b67c7728c84271bc00dc51272de643dafc270014079dcc9727b47577ba67b340aeb5f981588b54e69a06abea6958aa96
2022-08-02 14:06:03 -04:00
MacroFake
ddddd6913b
sort after scripted-diff 2022-08-02 15:31:05 +02:00
MacroFake
fac812ca83
scripted-diff: Move mempool_args to src/node
It is part of the node library. Also, it won't be moved to the kernel
lib, as it will be pruned of ArgsManager.

-BEGIN VERIFY SCRIPT-
 # Move module
 git mv src/mempool_args.cpp src/node/
 git mv src/mempool_args.h   src/node/
 # Replacements
 sed -i 's:mempool_args\.h:node/mempool_args.h:g'     $(git grep -l mempool_args)
 sed -i 's:mempool_args\.cpp:node/mempool_args.cpp:g' $(git grep -l mempool_args)
 sed -i 's:MEMPOOL_ARGS_H:NODE_MEMPOOL_ARGS_H:g'      $(git grep -l MEMPOOL_ARGS_H)
-END VERIFY SCRIPT-
2022-08-02 15:31:01 +02:00
MacroFake
66664384a6
Remove ::g_max_datacarrier_bytes global 2022-08-02 15:29:16 +02:00
MacroFake
fad0b4fab8
Pass datacarrier setting into IsStandard 2022-08-02 15:28:30 +02:00
MacroFake
fa2a6b8516
Combine datacarrier globals into one 2022-08-02 15:28:10 +02:00
MacroFake
fa477d32ee
Remove ::GetVirtualTransactionSize() alias
Each alias is only used in one place.
2022-08-02 15:27:20 +02:00
MacroFake
fa2f6c1a61
Remove ::fIsBareMultisigStd global 2022-08-02 15:27:19 +02:00
MacroFake
fadc14e4f5
Remove ::dustRelayFee 2022-08-02 15:26:49 +02:00
MacroFake
fa8a7f01fe
Remove ::IsStandardTx(tx, reason) alias
Apart from tests, it is only used in one place, so there is no need for
an alias.
2022-08-02 15:26:24 +02:00
MacroFake
fa7a9114e5
test: Remove unused cs_main 2022-08-02 15:25:10 +02:00
MacroFake
fa9cba7afb
Remove ::incrementalRelayFee and ::minRelayTxFee globals 2022-08-02 15:23:36 +02:00
MacroFake
fa148602e6
Remove ::fRequireStandard global 2022-08-02 15:23:24 +02:00
MacroFake
fa468bdfb6
Return optional error from ApplyArgsManOptions
Also pass in a (for now unused) reference to the params.

Both changes are needed for the next commit.
2022-08-02 15:21:50 +02:00
MacroFake
816ca01650
Merge bitcoin/bitcoin#25736: univalue: Remove unused and confusing set*() return value
fa7bef2e80c9c290b3e97114cfa7afdea5cbd53c univalue: Remove unused and confusing set*() return value (MacroFake)

Pull request description:

  The value is:

  * currently unused, and useless without `[[nodiscard]]`
  * confusing, because it is always `true`, unless a num-string is set

  Instead of adding `[[nodiscard]]`, throw when setting is not possible.

ACKs for top commit:
  shaavan:
    ACK fa7bef2e80c9c290b3e97114cfa7afdea5cbd53c
  aureleoules:
    ACK fa7bef2e80c9c290b3e97114cfa7afdea5cbd53c.

Tree-SHA512: 0d74f96f34cb93b66019ab75e12334c964630cc83434f22e58cc7a4fff2ee96a5767e42ab37f08acb67aeacba6811b09c75f1edc68d5e903ccfc59b1c82de891
2022-08-02 12:30:49 +02:00
MacroFake
da23320998
Merge bitcoin/bitcoin#25651: refactor: make all NodeImpl/ChainImpl/ExternalSignerImpl members public, rm temporaries, simplify
4bedfd702ad878645c51bea6ee8ce40d8c0bd3da refactor: remove unneeded temporaries in node/interfaces, simplify code (Jon Atack)
b27ba169ebd4a8e4ec29be590f03a4d0da61a0cc refactor: make all NodeImpl/ChainImpl/ExternalSignerImpl members public (Jon Atack)

Pull request description:

  - Make all `NodeImpl`, `ChainImpl` and `ExternalSignerImpl` class members `public` (and document why), to be consistent in all the `*Impl` classes in `src/node/interfaces.cpp` and `src/wallet/interfaces.cpp` and to help future reviewers and contributors.

  - Remove unneeded temporaries in `NodeImpl` and `ChainImpl` methods in `src/node/interfaces.cpp` and simplify, to make the code easier to read and understand and to improve performance by avoiding unnecessary move operations.

ACKs for top commit:
  ryanofsky:
    Code review ACK 4bedfd702ad878645c51bea6ee8ce40d8c0bd3da. Changes since last review, applying suggested style & simplifiying first commit. Also avoiding another lock in second commit.

Tree-SHA512: 112f7cad5e2838c94c5b79d61328f42fe75fdb97f401ab49eccf696fc2c6a8a0c0ee55ec974c0602acf7423f78bb82e90eb8a0cc531e1d3347f73b7c83685504
2022-08-01 11:19:55 +02:00