forked from bitcoin/bitcoin
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
backport: Merge bitcoin#23157, 24041, 20591, 22677 #6259
Draft
vijaydasmp
wants to merge
4
commits into
dashpay:develop
Choose a base branch
from
vijaydasmp:bp23_2024_09_04
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+405
−177
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vijaydasmp
changed the title
backport: Merge bitcoin#23157
backport: Merge bitcoin#23157, 19101
Sep 9, 2024
vijaydasmp
force-pushed
the
bp23_2024_09_04
branch
from
September 9, 2024 16:54
cd38ea9
to
4155253
Compare
vijaydasmp
changed the title
backport: Merge bitcoin#23157, 19101
backport: Merge bitcoin#23157, 24041, 22362, 23307, 23054
Sep 9, 2024
vijaydasmp
force-pushed
the
bp23_2024_09_04
branch
4 times, most recently
from
September 11, 2024 08:05
b9f947a
to
ebe61aa
Compare
vijaydasmp
changed the title
backport: Merge bitcoin#23157, 24041, 22362, 23307, 23054
backport: Merge bitcoin#23157, 24041, 22362, 23054
Sep 11, 2024
This pull request has conflicts, please rebase. |
vijaydasmp
force-pushed
the
bp23_2024_09_04
branch
from
September 11, 2024 15:17
710ce89
to
42d9210
Compare
This pull request has conflicts, please rebase. |
vijaydasmp
changed the title
backport: Merge bitcoin#23157, 24041, 22362, 23054
backport: Merge bitcoin#23157, 24041, 22362, 20591, 23054
Sep 14, 2024
vijaydasmp
changed the title
backport: Merge bitcoin#23157, 24041, 22362, 20591, 23054
backport: Merge bitcoin#23157, 24041, 22362, 20591, 22677
Sep 14, 2024
vijaydasmp
force-pushed
the
bp23_2024_09_04
branch
from
September 14, 2024 15:27
64caa39
to
6fce065
Compare
vijaydasmp
changed the title
backport: Merge bitcoin#23157, 24041, 22362, 20591, 22677
backport: Merge bitcoin#23157, 24041, 20591, 22677
Sep 14, 2024
vijaydasmp
force-pushed
the
bp23_2024_09_04
branch
2 times, most recently
from
September 15, 2024 08:14
12bae70
to
7f4b4f4
Compare
…ce of check() and remove dependency on validation 082c5bf [refactor] pass coinsview and height to check() (glozow) ed6115f [mempool] simplify some check() logic (glozow) 9e8d7ad [validation/mempool] use Spend/AddCoin instead of UpdateCoins (glozow) 09d1891 MOVEONLY: remove single-use helper func CheckInputsAndUpdateCoins (glozow) e8639ec [mempool] remove now-unnecessary code (glozow) 54c6f3c [mempool] speed up check() by using coins cache and iterating in topo order (glozow) 30e240f [bench] Benchmark CTxMemPool::check() (glozow) cb14071 [refactor/bench] make mempool_stress bench reusable and parameterizable (glozow) Pull request description: Remove the txmempool <-> validation circular dependency by removing txmempool's dependency on validation. There are two functions in txmempool that need validation right now: `check()` and `removeForReorg()`. This PR removes the dependencies in `check()`. This PR also improves the performance of `CTxMemPool::check()` by walking through the entries exactly once, in ascending ancestorcount order, which guarantees that we see parents before children. ACKs for top commit: jnewbery: reACK 082c5bf GeneFerneau: tACK [082c5bf](bitcoin@082c5bf) rajarshimaitra: tACK bitcoin@082c5bf theStack: Code-review ACK 082c5bf Tree-SHA512: 40ac622af1627b5c3e6abb4f0f035d833265a8c5e8dc88faf5354875dfb5137f137825e54bbd2a2668ed37b145c5d02285f776402629f58596e51853a9a79d29
b5c9bb5 util: Restore GetIntArg saturating behavior (James O'Beirne) Pull request description: The new locale-independent atoi64 method introduced in bitcoin#20452 parses large integer values higher than maximum representable value as 0 instead of the maximum value, which breaks backwards compatibility. This commit restores compatibility and adds test coverage for this case in terms of the related GetIntArg and strtoll functions. Specifically, command line or bitcoin.conf integer values greater than `9223372036854775807` (`2**63-1`) used to be parsed as `9223372036854775807` before bitcoin#20452. Then bitcoin#20452 caused them to be parsed as `0`. And after this PR they will be parsed as `9223372036854775807` again. This change is a stripped-down alternative version of bitcoin#23841 by jamesob ACKs for top commit: jamesob: Github ACK bitcoin@b5c9bb5 vincenzopalazzo: ACK bitcoin@b5c9bb5 MarcoFalke: review ACK b5c9bb5 🌘 Tree-SHA512: 4e8abdbabf3cf4713cf5a7c5169539159f359ab4109a4e7e644cc2e9b2b0c3c532fad9f6b772daf015e1c5340ce59280cd9a41f2730afda6099cbf636b7d23ae
…ring rescanning process. 240ea29 doc: update doxygen documention of ComputeTimeSmart() and AddToWalletIfInvolvingMe() regarding rescanning_old_block parameter (BitcoinTsunami) d6eb39a test: add functional test to check transaction time determination during block rescanning (BitcoinTsunami) 07b44f1 wallet: fix ComputeTimeSmart algorithm to use blocktime during old block rescanning (BitcoinTsunami) Pull request description: The function ComputeTimeSmart in wallet.cpp assume that transaction are discovered in the right order. Moreover the 'smarttime' determination algorithm is coded with realtime scenario in mind and not rescanning of old block. The functional test demonstrate that if the user import a wallet, then rescan only recent history, and then rescan the entire history, the older transaction discovered would have an incorrect time determination. In the context of rescanning old block, the only time value that as a meaning is the blocktime. That's why I've fixed the problem with a simple separation between rescanning of old block and realtime time determination. The fix is written to have no impact on every realtime scenario and only impact the behaviour during a rescanning process. This PR Fixes bitcoin#20181. To be fair, I don't think that this bug could be triggered with the wallet GUI, because it always proceed with a proper rescan. But RPC API provide the possibility to trigger it. I've discovered it, because Specter desktop v0.10.0 was impacted. (cryptoadvance/specter-desktop#680). ACKs for top commit: jonatack: ACK 240ea29 per `git diff b92d552 240ea29`, re-verified rebase to latest master + debug build clean + new test passes on the branch and fails on master, only change since my review a few hours ago is incorporation of latest review suggestions meshcollider: re-utACK 240ea29 Tree-SHA512: 514b02e41d011ddfa325f5e8080b93800e1ea4ed5853fa420670a6ac700e8b463000dbea65f8ced8565cfb950c7f51b69154034dcb111e67aca3b964a0061494
…ency 2/2 a64078e Break validation <-> txmempool circular dependency (glozow) 64e4963 [mempool] always assert coin spent (glozow) bb9078e [refactor] put finality and maturity checking into a lambda (glozow) bedf246 [mempool] only update lockpoints for non-removed entries (glozow) 1b3a11e MOVEONLY: TestLockPointValidity to txmempool (glozow) Pull request description: Remove 2 circular dependencies: validation - txmempool and validation - policy/rbf - txmempool Validation should depend on txmempool (e.g. `CChainstateManager` has a mempool and we often need to know what's in our mempool to validate transactions), but txmempool is a data structure that shouldn't really need to know about chain state. - Changes `removeForReorg()` to be parameterized by a callable that returns true/false (i.e. whether the transaction should be removed due to being now immature or nonfinal) instead of a `CChainState`. The mempool really shouldn't need to know about coinbase maturity or lockpoints, it just needs to know which entries to remove. ACKs for top commit: laanwj: Code review ACK a64078e mjdietzx: reACK a64078e theStack: re-ACK a64078e Tree-SHA512: f75995200569c09dfb8ddc09729da66ddb32167ff1e8a7e72f105ec062d2d6a9a390e6b4a2a115e7ad8ad3525f891ee1503f3cd2bed11773abcaf7c3230b1136
vijaydasmp
force-pushed
the
bp23_2024_09_04
branch
from
September 15, 2024 08:41
7f4b4f4
to
2856dca
Compare
Hi @UdjinM6 , This is stuck at ci/gitlab/gitlab.com Expected — Waiting for status to be reported |
Yes, this happens... try rebasing and force-pushing |
This pull request has conflicts, please rebase. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
backporting bitcoin