- store_path() previously only worked for cross compilation packages, we
remove this assumption here
- Add CROSS_GCC_LIB variable which points to where gcc libs/headers are
located
- Add gcc libs/headers to our CROSS_*_PATH environment variables
- Check that all directories in CROSS_*_PATH are sane
442a87cc0ae43ebc9b6654a6165778eecb931f74 Add a test wallet_reorgsrestore (Antoine Riard)
40ede992d97df38282919693dfe851c975c3b1d8 Modify wallet tx status if has been reorged out (Antoine Riard)
7e89994133725125dddbfa8d45484e3b9ed51c6e Remove SyncTransaction for conflicted txn in CWallet::BlockConnected (Antoine Riard)
a31be09bfd77eed497a8e251d31358e16e2f2eb1 Encapsulate tx status in a Confirmation struct (Antoine Riard)
Pull request description:
While working on #15931, I've tried to rationalize tx state management to ease integration of block height tracking per-wallet tx. We currently rely on a combination of `hashBlock` and `nIndex` with magic value to determine tx confirmation, conflicted or abandoned state. It's hard to reason and error-prone. To solve that, we encapsulate these fields in a `TxConfirmation` struct and introduce a `TxState` member that we update accordingly at block connection/disconnection.
Following jnewbery [recommendation](https://github.com/bitcoin/bitcoin/pull/15931#discussion_r312576506), I've taken these changes in its own commit, and open a PR to get them first. It would ease review of aforementioned PR, but above all should ease fixing of long-term issues like :
* https://github.com/bitcoin/bitcoin/issues/7315 (but maybe we should abandon abandontransaction or relieve it to only free outpoints not track the transaction as abandoned in itself, need its own discussion)
* https://github.com/bitcoin/bitcoin/issues/8692 where we should cancel conflicted state of transactions chain smoothly
* `MarkConflicted` in `LoadToWallet` is likely useless if we track conflicts rights at block connection
Main changes of this PR to get right are tx update in `AddToWallet` and serialization/deserialization logic.
ACKs for top commit:
meshcollider:
Light re-Code Review ACK 442a87cc0ae43ebc9b6654a6165778eecb931f74
ryanofsky:
utACK 442a87cc0ae43ebc9b6654a6165778eecb931f74. Changes since last review are switching from `hasChain` to `LockChain` and removing chain lock in `WalletBatch::LoadWallet` that's redundant with the new lock still added in `CWallet::LoadWallet`, and fixing python test race condition.
Tree-SHA512: 029209e006de0240436817204e69e548c5665e2b0721b214510e7aba7eba130a5eab441d3a1ad95bd6426114dd27390492c77bf4560a9610009b32cd0a1f72f7
2457aea83c1f9fba708e2335bb197950bf0b6244 Assert that the HRP is lowercase in Bech32::Encode (Samuel Dobson)
Pull request description:
From BIP-173:
> The lowercase form is used when determining a character's value for checksum purposes.
> Encoders MUST always output an all lowercase Bech32 string. If an uppercase version of the encoding result is desired, (e.g.- for presentation purposes, or QR code use), then an uppercasing procedure can be performed external to the encoding process.
Currently if HRP contains uppercase characters, the checksum will be generated over these uppercase characters resulting in mixed-case output that will always be invalid even if the case is changed manually after encoding. This shouldn't happen because both prefix's `bc` and `tb` are lowercase currently, but we assert this condition anyway.
This is consistent also with the [C reference implementation](2b0aac650c/ref/c/segwit_addr.c (L59))
ACKs for top commit:
laanwj:
ACK 2457aea83c1f9fba708e2335bb197950bf0b6244
Tree-SHA512: 24fcbbc2f315c72c550cc3d82b4332443eea6378fc73d571f98b87492604d023378dd102377c9e05467192cae6049606dee98e4c5688c8d5e4caac50c970284b
fae91a09c453a9a95c382df765bd71e54698d5b2 test: Remove incorrect and unused try-block in assert_debug_log (MarcoFalke)
Pull request description:
This try block has accidentally been added by me in fa3e9f7627784ee00980590e5bf044a0e1249999.
It was unused all the time, but commit 6011c9d72d1df5c2cd09de6f85c21eb4f7eb1ba8 added a `return` in the finally block, muting all exceptions.
This can be tested by adding an `assert False` after any `with ...assert_debug_log...:` line.
ACKs for top commit:
laanwj:
ACK fae91a09c453a9a95c382df765bd71e54698d5b2
ryanofsky:
utACK fae91a09c453a9a95c382df765bd71e54698d5b2. I didn't know returning inside a `finally` block would cancel pending exceptions or return values, but I guess this makes sense and is a good thing to be aware of.
Tree-SHA512: 47ed0165062060e9af055a3e92f1a529cd41d00476bfad64e3cd141ae084d22f926a343bb1257717e164e15459a59ab66aed198c95d18bf780d8cb0b76aa3298
3d0a82cff8cbb809876e82dbe62d14d2adc07d94 devtools: Accomodate block-style copyright blocks (Ben Woosley)
0ef0e51fe4bb592e67255776b5a0ba04679fb8c4 lint: Bump flake8 to 3.7.8 (Ben Woosley)
838920704ad90a71cf288b700052503db8abb17e lint: Disable flake8 W504 warning (Ben Woosley)
b21680baf5391a602b295b9d7d0ef66553661cb9 test/contrib: Fix invalid escapes in regex strings (Ben Woosley)
Pull request description:
This is a second go at #15221, fixing new lints in:
W504 line break after binary operator
W605 invalid escape sequence
F841 local variable 'e' is assigned to but never used
This time around:
* One commit per rule, for easier review
* I went with the PEP-8 style of breaking before binary operators
* I looked into the raw regex newline issue, and found that raw strings with newlines embedded do work appropriately. E.g. run `re.match(r" \n ", " \n ")` to check this for yourself. `re.MULTILINE` exists to modify `^` and `$` in multiline scenarios, but all of these searches are per-line.
ACKs for top commit:
practicalswift:
ACK 3d0a82cff8cbb809876e82dbe62d14d2adc07d94 -- diff looks correct
Tree-SHA512: bea0c144cadd72e4adf2e9a4b4ee0535dd91a8e694206924cf8a389dc9253f364a717edfe9abda88108fbb67fda19b9e823f46822d7303c0aaa72e48909a6105
490da639cbd48ce0dc438abbfc89ab796391cb2a Make lint-includes.sh work from any directory (Kristaps Kaupe)
Pull request description:
Before this change it works from root folder of bitcoin git repo, but if you do `cd test/lint; ./test-includes.sh`, you will have a lot of false positive messages like this:
```
Good job! The circular dependency "chainparamsbase -> util/system -> chainparamsbase" is no longer present.
Please remove it from EXPECTED_CIRCULAR_DEPENDENCIES in ./lint-circular-dependencies.sh
to make sure this circular dependency is not accidentally reintroduced.
Good job! The circular dependency "index/txindex -> validation -> index/txindex" is no longer present.
Please remove it from EXPECTED_CIRCULAR_DEPENDENCIES in ./lint-circular-dependencies.sh
to make sure this circular dependency is not accidentally reintroduced.
```
Top commit has no ACKs.
Tree-SHA512: 07fa69cb2883181dcee922191acac4b242722eeb2916cdffdc7163421302b22f3c9525aaf4c754a9dba1c307032c05285e38191d5c6aabc894321f8a27bbceaa
Accept RBF bumps of single transactions (ie which conflict with one
transaction) even when that transaction is a member of a package
which is currently at the package limit iff the new transaction
does not add any additional mempool dependencies from the original.
This could be made a bit looser in the future and still be safe,
but for now this fixes the case that a transaction which was
accepted by the carve-out rule will not be directly RBF'able.
If we set fRelay=false in our VERSION message, and a peer sends an INV or TX
message anyway, disconnect. Since we use fRelay=false to minimize bandwidth,
we should not tolerate remaining connected to a peer violating the protocol.
Transaction relay is primarily optimized for balancing redundancy/robustness
with bandwidth minimization -- as a result transaction relay leaks information
that adversaries can use to infer the network topology.
Network topology is better kept private for (at least) two reasons:
(a) Knowledge of the network graph can make it easier to find the source IP of
a given transaction.
(b) Knowledge of the network graph could be used to split a target node or
nodes from the honest network (eg by knowing which peers to attack in order to
achieve a network split).
We can eliminate the risks of (b) by separating block relay from transaction
relay; inferring network connectivity from the relay of blocks/block headers is
much more expensive for an adversary.
After this commit, bitcoind will make 2 additional outbound connections that
are only used for block relay. (In the future, we might consider rotating our
transaction-relay peers to help limit the effects of (a).)
dcc448e3d2b97f7e4e23f5ed174762cec3530306 Avoid unnecessary "Synchronizing blockheaders" log messages (Jonas Schnelli)
Pull request description:
Fixes#16773
I'm not entirely sure why 16773 happend, but probably due to headers fallback in a compact block.
However, this PR should fix it and should have been included in #15615.
ACKs for top commit:
ajtowns:
ACK dcc448e3d2b97f7e4e23f5ed174762cec3530306 ; code review only, haven't compiled or tested.
promag:
ACK dcc448e3d2b97f7e4e23f5ed174762cec3530306.
TheBlueMatt:
utACK dcc448e3d2b97f7e4e23f5ed174762cec3530306. Went and read how pindexBestHeader is handled and this code looks correct (worst case it breaks a LogPrint, so whatever). I also ran into this on #16762.
fanquake:
ACK dcc448e3d2b97f7e4e23f5ed174762cec3530306
Tree-SHA512: f8cac3b6eb9d4e8fab53a535b55f9ea9b058e3ab6ade64801ebc56439ede4f54b5fee36d5d2b316966ab987b65b13ab9dc18849f345d08b81ecdf2722a3f5f5a
Flagged by flake8 v3.6.0, as W605, plus a few others identified
incidentally, e.g. 59ffecf66cf4d08c4b431e457b083878d66a3fd6.
Note that r"\n" matches to "\n" under re.match/search.
fa61365a1368f9970fa22fb96f89f4ecc08e69f0 wallet: Translate all initErrors in CreateWalletFromFile (MarcoFalke)
fa70d199d0c2182d76b2a1cfa21f9ada4bb12913 util: Make util/error bilingual_str (refactor) (MarcoFalke)
Pull request description:
The translations are going to close in three days (#15940), so I am submitting this as a standalone pull request.
Those changes are part of a bugfix #16661, which includes a test. The first change (the refactor) is required, the second commit is not. I am happy to drop it, if needed.
ACKs for top commit:
laanwj:
utACK fa61365a1368f9970fa22fb96f89f4ecc08e69f0
hebasto:
ACK fa61365a1368f9970fa22fb96f89f4ecc08e69f0, I have tested the code on Linux Mint 19.2.
Tree-SHA512: a7616cc38b9ffd301c6b915ea808a65815c3d97e9f57ec091772eb260e5cf0d75a13a6e4dfa3913e236833677c7929b9a748cb7d7a0e406d51749944b614e11b
057fac345307a70a503ae47ac478ae49a7208fe6 gui: Update English source translation (Wladimir J. van der Laan)
072f37a7ed1141a1f41eca22ac283834d2c3f6da gui: Update transifex slug for 0.19 (Wladimir J. van der Laan)
Pull request description:
- Update transifex slug for 0.19
- Update English source translation
ACKs for top commit:
fanquake:
ACK 057fac345307a70a503ae47ac478ae49a7208fe6 - I see the same diff when running `make translate`.
Tree-SHA512: e6800011c2cc642b656d2a8fa652e94c665183424321eb4f1f3506e90a65f730794e5e9d49243538c0710c66fec1ed6f30e099dbc8a847e840817e894782866f
9965940e35c445ccded55510348af228ff22f0e9 doc: Add release note for the new gettransaction argument (darosior)
b8b3f0435a2837d3897e9e232ef6ca839ce74eb8 tests: Add a new functional test for gettransaction (darosior)
7f3bb247a811582d1aa4805d8e601c19808dc7ba gettransaction: add an argument to decode the transaction (darosior)
Pull request description:
This PR adds a new parameter to the `gettransaction` call : `decode`. If set to `true`, it will add a new `decoded` field to the response. This mimics the behavior of `getrawtransaction`'s `verbose` argument to avoid using 2 calls if we want to decode a wallet transaction (`gettransaction` then `decoderawtransaction`).
Fix#16181 .
ACKs for top commit:
meshcollider:
re-utACK 9965940e35c445ccded55510348af228ff22f0e9
Tree-SHA512: bcb6b4bd252b3488d6afc77659c499c2ad99fd58661eb24b6a2e17014c74f22e47fde70e00fedb4f4754915786622ad02483b2cf2c4dea0ab0eb4ac8276dbeee
9b92538adebd30353ffc1a427bc20ab0c3fc75f1 Remove unused fScriptChecks parameter from CheckInputs (Matt Corallo)
Pull request description:
fScriptChecks = false just short-circuits the entire function, so
passing it in is entirely useless.
This is extracted from #13233 /cc TheBlueMatt.
Recommend reviewing with `git show --ignore-all-space`, i.e.:
https://github.com/bitcoin/bitcoin/pull/13868/files?w=1
ACKs for top commit:
TheBlueMatt:
utACK 9b92538adebd30353ffc1a427bc20ab0c3fc75f1. Checked diff had no functional change and new comment copy looks correct.
kallewoof:
ACK 9b92538adebd30353ffc1a427bc20ab0c3fc75f1
ajtowns:
ACK 9b92538adebd30353ffc1a427bc20ab0c3fc75f1 ; code review, checked tests work. Looks right to me, and fanquake's notes make sense. Could change the coinbase early exit to `assert(!tx.IsCoinBase());`.
fanquake:
ACK 9b92538adebd30353ffc1a427bc20ab0c3fc75f1 - Notes / testing below.
Tree-SHA512: add253a3e8cf4b33eddbc49efcec333c14b5ea61c7d34e43230351d40cff6adc919a75b91c72c4de8647a395284db74a61639f4c67848d4b2fec3a705b557790
0bb33b5348dbddd65b88a7f00449c965562355d3 qt: Replace objc_msgSend with native syntax (Hennadii Stepanov)
Pull request description:
Changes in Xcode 11 Objective-C Runtime cause an error (#16387) during building on MacOS 10.15 Catalina.
This PR fixes this issue by replacing `objc_msgSend()` function calls with the native Objective-C syntax.
Refs:
- [changes in `objc_msgSend` function](https://developer.apple.com/documentation/objectivec/1456712-objc_msgsend?changes=latest_minor&language=objc)
- [`OBJC_OLD_DISPATCH_PROTOTYPES` macro](https://developer.apple.com/documentation/objectivec/objc_old_dispatch_prototypes?language=objc)
ACKs for top commit:
l2a5b1:
ACK 0bb33b5 - Diff looks good. Sending messages via native Objective-C code feels more robust and is more readable than casting all the `objc_msgSend` function calls to the appropriate function signature (which would also have fixed the issue).
jonasschnelli:
utACK 0bb33b5348dbddd65b88a7f00449c965562355d3 - Confirmed that the called macOS framework function is available on our build targets.
fanquake:
ACK 0bb33b5348dbddd65b88a7f00449c965562355d3 - Still works as expected.
Tree-SHA512: c09cb684d06bd1da053a17c182b7bb1642e45bb347d26c76e1c5d835c320567caee366d85e34bb7f2be38e63ed041e0d06a56c2a9d89f7e5bece9b19cc5c6772
8340763dc3278c828ee7d525de4c71b4615d17cc Check for codespell in lint-spelling.sh (Kristaps Kaupe)
Pull request description:
Similar check for `spellcheck` already exists in `lint-shell.sh`
ACKs for top commit:
practicalswift:
ACK 8340763dc3278c828ee7d525de4c71b4615d17cc -- diff looks correct
fanquake:
ACK 8340763dc3278c828ee7d525de4c71b4615d17cc - Test is the same as in `lint-shell.sh`.
Tree-SHA512: f590fc4eae391c5246e86cb70cf79431bd3763ac872f82d3b2222c3a095235ca046a7072c5e61bbaeb3ed6ecd5ff85979769ee8b585de4fb97ac8cfbbf73b956
d9d8984270dbb004ec94f8dbb289be2bc9e4dbc3 wallet: Use wallet name instead of pointer on unload/release (João Barbosa)
Pull request description:
Fixes#16668. Wallet name is unique so it can be used instead of pointer.
ACKs for top commit:
meshcollider:
utACK d9d8984270dbb004ec94f8dbb289be2bc9e4dbc3
instagibbs:
utACK d9d8984270
ryanofsky:
utACK d9d8984270dbb004ec94f8dbb289be2bc9e4dbc3. Alternately I think it might be possible to use an intptr_t set instead of a string set to get around the undefined behavior described in the issue.
Tree-SHA512: eccd4d260cd4c02b52c30deeb32dbfd190a1151a5340eb3aa4ece0dc6ae3b3ed746ce5617336461f6f27c437c435629cd07d20beb1c5450f23b75edde6728598
635e9154da223ca760713d7b3b66c0a9321d1277 [qt] Change uninstall icon on Windows (GChuf)
Pull request description:
Change uninstall icon in Windows by changing a registry value.
Original uninstall.exe icon remains the same
Reason: almost no other modern program uses that uninstall icon in Windows.
before:
after:
ACKs for top commit:
promag:
ACK 635e9154da223ca760713d7b3b66c0a9321d1277.
l2a5b1:
ACK 635e915
practicalswift:
ACK 635e9154da223ca760713d7b3b66c0a9321d1277
jonasschnelli:
utACK 635e9154da223ca760713d7b3b66c0a9321d1277
fanquake:
ACK 635e9154da223ca760713d7b3b66c0a9321d1277 - Tested building and installing using WSL on Windows 10.
Tree-SHA512: a0f435c79e726896cc93db058f1712dcf37daefbcacbf213b340ef2d5f4ff387148b8d659302f3e84be8a9f3df23e72b0ca5937f5d77499b808081bd40bfbbac
