aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2019-09-07Merge #15759: p2p: Add 2 outbound block-relay-only connectionsfanquake
0ba08020c9791f7caf5986ad6490c16a2b66cd83 Disconnect peers violating blocks-only mode (Suhas Daftuar) 937eba91e1550bc3038dc541c236ac83e0a0e6d5 doc: improve comments relating to block-relay-only peers (Suhas Daftuar) 430f489027f15c1e4948ea4378954df24e3fee88 Don't relay addr messages to block-relay-only peers (Suhas Daftuar) 3a5e885306ea954d7eccdc11502e91a51dab8ec6 Add 2 outbound block-relay-only connections (Suhas Daftuar) b83f51a4bbe29bf130a2b0c0e85e5bffea107f75 Add comment explaining intended use of m_tx_relay (Suhas Daftuar) e75c39cd425f8c4e5b6bbb2beecb9c80034fefe1 Check that tx_relay is initialized before access (Suhas Daftuar) c4aa2ba82211ea5988ed7fe21e1b08bc3367e6d4 [refactor] Change tx_relay structure to be unique_ptr (Suhas Daftuar) 4de0dbac9b286c42a9b10132b7c2d76712f1a319 [refactor] Move tx relay state to separate structure (Suhas Daftuar) 26a93bce29fd813e1402b013f402869c25b656d1 Remove unused variable (Suhas Daftuar) Pull request description: Transaction relay is optimized for a combination of redundancy/robustness as well as 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).) ACKs for top commit: sipa: ACK 0ba08020c9791f7caf5986ad6490c16a2b66cd83 ajtowns: ACK 0ba08020c9791f7caf5986ad6490c16a2b66cd83 -- code review, ran tests. ran it on mainnet for a couple of days with MAX_BLOCKS_ONLY_CONNECTIONS upped from 2 to 16 and didn't observe any unexpected behaviour: it disconnected a couple of peers that tried sending inv's, and it successfully did compact block relay with some block relay peers. TheBlueMatt: re-utACK 0ba08020c9791f7caf5986ad6490c16a2b66cd83. Pointed out that stats.fRelayTxes was sometimes uninitialized for blocksonly peers (though its not a big deal and only effects RPC), which has since been fixed here. Otherwise changes are pretty trivial so looks good. jnewbery: utACK 0ba08020c9791f7caf5986ad6490c16a2b66cd83 jamesob: ACK https://github.com/bitcoin/bitcoin/commit/0ba08020c9791f7caf5986ad6490c16a2b66cd83 Tree-SHA512: 4c3629434472c7dd4125253417b1be41967a508c3cfec8af5a34cad685464fbebbb6558f0f8f5c0d4463e3ffa4fa3aabd58247692cb9ab8395f4993078b9bcdf
2019-09-07Merge #15450: gui: Create wallet menu optionfanquake
613de61a04c210a51af9997e69f66439a17a632a Add Create Wallet menu action (Andrew Chow) 9b41cbb28f603f4f71f5854d6ae2527932bba3cb Expose wallet creation to the GUI via WalletController (Andrew Chow) 78863e290006e61060622dbdbecc5b58c0fefa05 Add CreateWalletDialog to create wallets from the GUI (Andrew Chow) 60adb21c7affb41ec475a62a53fb0a36bea35dfb Optionally allow AskPassphraseDialog to output the passphrase (Andrew Chow) bc6d8a3662f0fb992073e5e80269a90a722d76e0 gui: Refactor OpenWalletActivity (João Barbosa) Pull request description: This PR adds a menu option to create a new wallet. When clicked, a `CreateWalletDialog` will be created and prompt the user to name the wallet and choose whether to disable private keys, make a blank wallet, and encrypt the wallet. If the wallet is encrypted, the wallet will be born encrypted with the wallet first created blank, then encrypted, and then a new HD seed generated and set. To allow the newly created wallets to be encrypted, some changes to how encrypting a wallet works. Instead of encrypting and locking the wallet, the wallet will be encrypted and then unlocked. This is also an extra belt-and-suspenders check to make sure that encryption worked. ACKs for top commit: fanquake: ACK 613de61a04c210a51af9997e69f66439a17a632a - re-reviewed on macOS. I'm going to merge this now. It's had a stack of review, and as mentioned multiple times above, lets get this into `master` so it can get more testing pre `v0.19.0`. Tree-SHA512: 3f22cc20b13703ffc90d366ae9133114832fea77f4f319da7fd85eb454f2f0bd5d7e1e6e20284dea2f370d8574f83b45669dcbbe506b994410d32e8e7a6fa877
2019-09-07Merge #16421: Conservatively accept RBF bumps bumping one tx at the package ↵fanquake
limits 5ce822efbe45513ce3517c1ca731ac6d6a0c3b54 Conservatively accept RBF bumps bumping one tx at the package limits (Matt Corallo) Pull request description: Based on #15681, this adds support for some simple cases of RBF inside of large packages. Issue pointed out by sdaftuar in #15681, and this fix (or a broader one) is required ot make #15681 fully useful. Accept RBF bumps of single transactions (ie which evict exactly 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 ACKs for top commit: instagibbs: re-ACK https://github.com/bitcoin/bitcoin/pull/16421/commits/5ce822efbe45513ce3517c1ca731ac6d6a0c3b54 ajtowns: ACK 5ce822efbe45513ce3517c1ca731ac6d6a0c3b54 ; GetSizeWithDescendants is only change and makes sense sipa: Code review ACK 5ce822efbe45513ce3517c1ca731ac6d6a0c3b54. I haven't thought hard about the effect on potential DoS issues this policy change may have. Tree-SHA512: 1cee3bc57393940a30206679eb60c3ec8cb4f4825d27d40d1f062c86bd22542dd5944fa5567601c74c8d9fd425333ed3e686195170925cfc68777e861844bd55
2019-09-07Merge #16798: Refactor rawtransaction_util's SignTransaction to separate ↵fanquake
prevtx parsing 39034f1ee628dae0bc9da5b1b30b8a424e66d968 Refactor rawtransaction_util's SignTransaction to have previous tx parsing be separate (Andrew Chow) Pull request description: Currently the `SignTransaction` function has to handle both the actual signing and parsing of previous transaction data. This PR splits it so that `SignTransaction` only handles the signing itself and adds a `ParsePrevouts` function which handles parsing the prevtx information. This allows for `SignTransaction` to just take any `SigningProvider`. Split from #16341 ACKs for top commit: MarcoFalke: ACK 39034f1ee628dae0bc9da5b1b30b8a424e66d968 instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/16798/commits/39034f1ee628dae0bc9da5b1b30b8a424e66d968 ryanofsky: utACK 39034f1ee628dae0bc9da5b1b30b8a424e66d968. No change since previously reviewed b49bbb939be92a67ff77c3f7bca5bb94dd141906, https://github.com/bitcoin/bitcoin/pull/16341#pullrequestreview-278610269 other than rebase with no conflicts. Tree-SHA512: 09f7733e90691766bfb5cf0f20e913dbf270bd3b51abdcad966b24d110e562ed85fd3d0d1d7bbea61f903340060052ec73c4817b09aee0dc1f3916d781a9e40c
2019-09-06Merge #16793: refactor: Avoid locking cs_main in ProcessNewBlockHeadersMarcoFalke
3109a1f948f3c8fd500defbdc4e59bfb2953c30b refactor: Avoid locking cs_main in ProcessNewBlockHeaders (João Barbosa) Pull request description: Builds on #16774, this change avoids locking `cs_main` in `ProcessNewBlockHeaders` when the tip has changed - in this case the removed lock was necessary to just log a message. Top commit has no ACKs. Tree-SHA512: 31be6d319fa122804f72fa813cec5ed041dd7e4aef3c1921124a1f03016925c43cd4d9a272d80093e77fa7600e3506ef47b7bb821afcbffe01e6be9bceb6dc00
2019-09-05Add Create Wallet menu actionAndrew Chow
Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
2019-09-05Expose wallet creation to the GUI via WalletControllerAndrew Chow
Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
2019-09-05Add CreateWalletDialog to create wallets from the GUIAndrew Chow
Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
2019-09-05Optionally allow AskPassphraseDialog to output the passphraseAndrew Chow
2019-09-06refactor: Avoid locking cs_main in ProcessNewBlockHeadersJoão Barbosa
2019-09-06gui: Refactor OpenWalletActivityJoão Barbosa
2019-09-06Merge #16624: wallet: encapsulate transactions stateMeshCollider
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
2019-09-05Merge #16792: Assert that the HRP is lowercase in Bech32::EncodeWladimir J. van der Laan
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](https://github.com/sipa/bech32/blob/2b0aac650ce560fb2b2a2bebeacaa5c87d7e5938/ref/c/segwit_addr.c#L59) ACKs for top commit: laanwj: ACK 2457aea83c1f9fba708e2335bb197950bf0b6244 Tree-SHA512: 24fcbbc2f315c72c550cc3d82b4332443eea6378fc73d571f98b87492604d023378dd102377c9e05467192cae6049606dee98e4c5688c8d5e4caac50c970284b
2019-09-05Assert that the HRP is lowercase in Bech32::EncodeSamuel Dobson
2019-09-04Conservatively accept RBF bumps bumping one tx at the package limitsMatt Corallo
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.
2019-09-04Disconnect peers violating blocks-only modeSuhas Daftuar
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.
2019-09-04doc: improve comments relating to block-relay-only peersSuhas Daftuar
2019-09-04Don't relay addr messages to block-relay-only peersSuhas Daftuar
We don't want relay of addr messages to leak information about these network links.
2019-09-04Add 2 outbound block-relay-only connectionsSuhas Daftuar
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).)
2019-09-04Add comment explaining intended use of m_tx_relaySuhas Daftuar
2019-09-04Check that tx_relay is initialized before accessSuhas Daftuar
2019-09-03Merge #16774: Avoid unnecessary "Synchronizing blockheaders" log messagesMarcoFalke
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
2019-09-03Refactor rawtransaction_util's SignTransaction to have previous tx parsing ↵Andrew Chow
be separate
2019-09-03Merge #16745: wallet: Translate all initErrors in CreateWalletFromFileWladimir J. van der Laan
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
2019-09-02gui: Update English source translationWladimir J. van der Laan
2019-09-02Merge #16185: gettransaction: add an argument to decode the transactionMeshCollider
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
2019-09-02Merge #13868: Remove unused fScriptChecks parameter from CheckInputsfanquake
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
2019-09-01Merge #16720: qt: Replace objc_msgSend() function calls with the native ↵fanquake
Objective-C syntax 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
2019-08-31Avoid unnecessary "Synchronizing blockheaders" log messagesJonas Schnelli
2019-08-31qt: Replace objc_msgSend with native syntaxHennadii Stepanov
2019-08-31Merge #16716: wallet: Use wallet name instead of pointer on unload/releasefanquake
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 https://github.com/bitcoin/bitcoin/pull/16716/commits/d9d8984270dbb004ec94f8dbb289be2bc9e4dbc3 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
2019-08-30gettransaction: add an argument to decode the transactiondarosior
This adds a new boolean parameter 'decode' to the gettransaction call, which, if set to true, add a 'decoded' field to the result containing the decoded transaction
2019-08-30Merge #16758: qt: Replace QFontMetrics::width() with TextWidth()fanquake
8b6f5aabb9b21bea875170ae0f49661a4e38dbd3 qt: Replace QFontMetrics::width() with TextWidth() (Hennadii Stepanov) Pull request description: Compiling master (d8fc9979130a268cf55b12c223b20fd7d4b9f67f) on macOS Catalina (with a patch from #16720) reveals one more instance of `QFontMetrics::width()` which is supposed to be replaced with `TextWidth()` in the merged #16701. Sorry for incomplete solution provided in #16701. It’s especially sad that the line I missed lies in only 7 lines from the code touched in #16701. ACKs for top commit: fanquake: ACK 8b6f5aabb9b21bea875170ae0f49661a4e38dbd3 Tree-SHA512: 65cd8bea550150e5ee47c1e906d8c2393547cf4feba3701a933a4f24fad5ecdb552ac2de4e1200ed14efaa0df0480150dd58fccbddc3b902f6c2141603874902
2019-08-30Merge #16753: wallet: extract PubKey from P2PK script with Solverfanquake
798a589aff64b83a0844688a661f4bd987c3340c wallet: extract PubKey from P2PK script with Solver (Sebastian Falbesoner) Pull request description: The function `ExtractPubKey()` checks if a given script matches the P2PK pattern (`<PubKey> OP_CHECKSIG`), extracts the PubKey and additionally checks if it is cryptographically valid (full validation with ECC library via `CPubKey::IsFullyValid()`). Currently this is done manually in the following order: 1. check if first script OP is data push with valid PubKey length (first part of pattern match), extract PubKey 2. create `CPubKey` object with extracted PubKey 3. fully validate public key 4. check if last script OP is `OP_CHECKSIG` (second part of pattern match) Using Solver, the pattern matching and PubKey extraction can be done via a single step, leading to the following simplified order with shorter code: 1. check if given script matches P2PK pattern with Solver (also contains valid PubKey length check), extracts Pubkey 2. create `CPubKey` object with extracted Pubkey 3. fully validate public key ACKs for top commit: instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/16753/commits/798a589aff64b83a0844688a661f4bd987c3340c theStack: > utACK [798a589](https://github.com/bitcoin/bitcoin/commit/798a589aff64b83a0844688a661f4bd987c3340c) sipa: ACK 798a589aff64b83a0844688a661f4bd987c3340c achow101: Code Review ACK 798a589aff64b83a0844688a661f4bd987c3340c Tree-SHA512: 350358a89afed8c2a7967c50e9714a2d4a909259b50e694ce68dde3e7d0fa0bf3238d33642e73f2bdb53860f6d3f7327ca3eb6426b74eaffacfbca0a384d68cd
2019-08-30Merge #14862: doc: Declare BLOCK_VALID_HEADER reservedfanquake
fa0b910486e9aada077fe47e1201dcb3bd523c87 [doc] chain: Declare BLOCK_VALID_HEADER reserved (MarcoFalke) Pull request description: `BLOCK_VALID_HEADER` was never used and the comment is confusing to me in several ways: * It claims "version ok". However, without the previous header, it is not possible to check the validity of the version since the height needs to be known (c.f. BIP 90) * It claims "hash satisfies claimed PoW". While it is possible to check against the claimed PoW, it is not possible without the previous header to check that the claimed PoW is itself valid. * It claims "1 <= vtx count <= max". However, with the header alone and current consensus rules, the number of transactions is unknown. ACKs for top commit: sipa: ACK fa0b910486e9aada077fe47e1201dcb3bd523c87 ryanofsky: ACK fa0b910486e9aada077fe47e1201dcb3bd523c87 Tree-SHA512: 3972995a0a2f83aa55767bf8982af1fcb9493483f62aee6df27e58be9181a48d5968ae718b390cecc8be3ed4f26495683b1cffde8ef272dea0bd610ec169ef8b
2019-08-29Merge #16707: qt: Remove obsolete QModelIndex::child()Jonas Schnelli
c53667764ea65c0f8661358759b0124f808ce587 qt: Remove obsolete QModelIndex::child() (Hennadii Stepanov) Pull request description: The `QModelIndex::child()` member function is [obsolete](https://doc.qt.io/qt-5.12/qmodelindex-obsolete.html) since Qt 5.12. This PR removes it, does not change behavior and keeps compatibility with [Qt 5.5.1](https://github.com/bitcoin/bitcoin/pull/15393). Here is an excerpt from the master build log: ``` qt/receivecoinsdialog.cpp: In member function ‘void ReceiveCoinsDialog::copyColumnToClipboard(int)’: qt/receivecoinsdialog.cpp:264:111: warning: ‘QModelIndex QModelIndex::child(int, int) const’ is deprecated: Use QAbstractItemModel::index [-Wdeprecated-declarations] GUIUtil::setClipboard(model->getRecentRequestsTableModel()->data(firstIndex.child(firstIndex.row(), column), Qt::EditRole).toString()); ^ In file included from /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/qabstractitemview.h:45:0, from /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/qheaderview.h:44, from /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/QHeaderView:1, from ./qt/guiutil.h:12, from ./qt/receivecoinsdialog.h:8, from qt/receivecoinsdialog.cpp:7: /home/hebasto/Qt/5.13.0/gcc_64/include/QtCore/qabstractitemmodel.h:457:20: note: declared here inline QModelIndex QModelIndex::child(int arow, int acolumn) const ^~~~~~~~~~~ ``` ACKs for top commit: laanwj: Code review ACK c53667764ea65c0f8661358759b0124f808ce587 promag: ACK c53667764ea65c0f8661358759b0124f808ce587, just read the change. jonasschnelli: utACK c53667764ea65c0f8661358759b0124f808ce587 Tree-SHA512: 99fcb6ff60a6d47b925bda9f14006269eaad09ba4f7a41ac4975c6cf04bd906b58aed721cbfa0be7da8e6613d92e30d4be18b7e4d3960f026c7226558a4c3196
2019-08-29qt: Replace QFontMetrics::width() with TextWidth()Hennadii Stepanov
2019-08-29Modify wallet tx status if has been reorged outAntoine Riard
Add a LockChain method to CWallet to know if we can lock or query chain state safely. At tx loading, we rely on chain to know if hashBlock of tx is still in main chain. If not, we set its status to unconfirmed and reset its hashBlock/nIndex. If wallet loaded is the wallet-tool one, all wallet txn will show up with a height of zero. It doesn't matter as status is not used by wallet-tool. We take lock prematurely in CWallet::LoadWallet and CWallet::Verify to ensure that lock order is respected between cs_main an cs_wallet.
2019-08-29Merge #16695: rpc: Add window final block height to getchaintxstatsWladimir J. van der Laan
d48c1e837ae1bd08e0f18ad1b57ff72675c3d6ad Add window final block height to getchaintxstats (Jonathan "Duke" Leto) Pull request description: This patch is motivated by the desire to make the output of `getchaintxstats` more useful and optimized for applications to consume and render the data. Firstly, this data is already available to the RPC, no additional work is done. Currently additional RPC calls will be needed to look up the height of the final block in the window or the block height that began the window. By adding the block height of the final block in the window, the JSON is "self-contained" and applications can calculate the exact block height range of the window with no additional RPC requests. For example, a web application which wants to render historical information for `getchaintxstats` RPC on various window sizes might call the RPC with various window lengths, once per day, and store the JSON results somewhere. Because the final block height of each dataset is included, it's no extra work to determine the exact block window range of each JSON response. ACKs for top commit: promag: ACK d48c1e837ae1bd08e0f18ad1b57ff72675c3d6ad. Tree-SHA512: fd4952c125f81a4ad18f7c78498c6b3e265b93cb574832166ac25596321ce84957f971f3f78f37d7e42638dc65f2a5d4d760f289873c9c2f2a82eb00a0f87c3f
2019-08-29Merge #16706: qt: Replace deprecated QSignalMapper by lambda expressionsWladimir J. van der Laan
091213403922e970b38cc3a98c11074e02ddba14 qt: Remove QSignalMapper from TransactionView (Hennadii Stepanov) 9e0c1d676c5892fb5bd8fe98781915506a25108f qt: Remove QSignalMapper from RPCConsole (Hennadii Stepanov) Pull request description: The [`QSignalMapper`](https://doc.qt.io/qt-5/qsignalmapper.html) class has been [deprecated](https://doc-snapshots.qt.io/qt5-5.10/obsoleteclasses.html) since Qt 5.10. This PR replaces it by lambdas and does not change behavior. ACKs for top commit: jonasschnelli: utACK 091213403922e970b38cc3a98c11074e02ddba14 Tree-SHA512: 0c102d5cab4adc8b6252f72e07123ac87c65434c88cada3e72816ecea8fc4803f15b9c050fb5e1c7e8a96f709265521fd6813ab1890dbf5634032f7ee0d50675
2019-08-29Merge #15615: Add log output during initial header syncWladimir J. van der Laan
d75e704ac08fb43320e3ebcdde0d2a5392fdec9f Add log output during initial header sync (Jonas Schnelli) Pull request description: The non debug log output is completely quiet during the header sync. I see two main reasons to add infos about the state of the initial header sync... * users may think the node did fail to start sync * it's a little complicate to check if your getting throttled during header sync (repeatedly calling `getchaintips` or similar) ACKs for top commit: fanquake: Concept ACK https://github.com/bitcoin/bitcoin/pull/15615/commits/d75e704ac08fb43320e3ebcdde0d2a5392fdec9f practicalswift: utACK d75e704ac08fb43320e3ebcdde0d2a5392fdec9f laanwj: Tested ACK d75e704ac08fb43320e3ebcdde0d2a5392fdec9f Tree-SHA512: 2e738571b703d7251290864603c3a829729645962c2fa3187250bab0585e66a5f01fce892e9b5b98da451fab2b40a2e4784f9b2e5a9cad75ff62c535affe7430
2019-08-29Add window final block height to getchaintxstatsJonathan "Duke" Leto
The getchaintxstats RPC now returns the additional key of window_final_block_height
2019-08-29Merge #16701: qt: Replace functions deprecated in Qt 5.13Wladimir J. van der Laan
c6dd32da697e5a8052cbabe8c7605d27c43a8dfb qt: Replace obsolete functions of QDesktopWidget (Hennadii Stepanov) 1260ecd812e35185898fd555ad3e01d019072bcf qt: Add TextWidth() wrapper (Hennadii Stepanov) Pull request description: The following functions are obsolete in Qt 5.13: - [`QFontMetrics::width()`](https://doc.qt.io/qt-5/qfontmetrics-obsolete.html#width) - [`QDesktopWidget::availableGeometry()`](https://doc.qt.io/qt-5/qdesktopwidget-obsolete.html#availableGeometry) - [`QDesktopWidget::screenGeometry()`](https://doc.qt.io/qt-5/qdesktopwidget-obsolete.html#screenGeometry) This PR replaces them and does not change behavior. Here are some excerpts from the master build log: ``` qt/bitcoingui.cpp: In constructor ‘BitcoinGUI::BitcoinGUI(interfaces::Node&, const PlatformStyle*, const NetworkStyle*, QWidget*)’: qt/bitcoingui.cpp:84:57: warning: ‘const QRect QDesktopWidget::availableGeometry(int) const’ is deprecated: Use QGuiApplication::screens() [-Wdeprecated-declarations] move(QApplication::desktop()->availableGeometry().center() - frameGeometry().center()); ^ In file included from /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/QDesktopWidget:1:0, from qt/bitcoingui.cpp:43: /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/qdesktopwidget.h:88:67: note: declared here QT_DEPRECATED_X("Use QGuiApplication::screens()") const QRect availableGeometry(int screen = -1) const; ^~~~~~~~~~~~~~~~~ ``` ``` qt/bitcoingui.cpp:1410:74: warning: ‘int QFontMetrics::width(const QString&, int) const’ is deprecated: Use QFontMetrics::horizontalAdvance [-Wdeprecated-declarations] max_width = qMax(max_width, fm.width(BitcoinUnits::longName(unit))); ^ In file included from /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/qwidget.h:50:0, from /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/qdialog.h:44, from /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/QDialog:1, from ./qt/optionsdialog.h:8, from ./qt/bitcoingui.h:12, from qt/bitcoingui.cpp:5: /home/hebasto/Qt/5.13.0/gcc_64/include/QtGui/qfontmetrics.h:108:9: note: declared here int width(const QString &, int len = -1) const; ^~~~~ ``` ``` qt/splashscreen.cpp: In constructor ‘SplashScreen::SplashScreen(interfaces::Node&, Qt::WindowFlags, const NetworkStyle*)’: qt/splashscreen.cpp:127:50: warning: ‘const QRect QDesktopWidget::screenGeometry(int) const’ is deprecated: Use QGuiApplication::screens() [-Wdeprecated-declarations] move(QApplication::desktop()->screenGeometry().center() - r.center()); ^ In file included from /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/QDesktopWidget:1:0, from qt/splashscreen.cpp:24: /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/qdesktopwidget.h:79:67: note: declared here QT_DEPRECATED_X("Use QGuiApplication::screens()") const QRect screenGeometry(int screen = -1) const; ^~~~~~~~~~~~~~ ``` ACKs for top commit: jonasschnelli: utACK c6dd32da697e5a8052cbabe8c7605d27c43a8dfb Tree-SHA512: deb7bcbf86e1dcc6508bd91288772c2fe8811db79fa2011de37d0469cdd094fbf7fd8c4512c607bed0bd08dc2968e893c0bbc190732c43c69ed1085259df766c
2019-08-29wallet: extract PubKey from P2PK script with SolverSebastian Falbesoner
The function ExtractPubKey() checks if a given script matches the P2PK pattern (<PubKey> OP_CHECKSIG), extracts the PubKey and additionally checks if it is cryptographically valid (full validation with ECC library via .IsFullyValid()). Currently this is done manually in the following order: 1) check if first script OP is data push with valid PubKey length (first part of pattern match), extract PubKey 2) create CPubKey object with extracted PubKey 3) fully validate public key 4) check if last script OP is OP_CHECKSIG (second part of pattern match) Using Solver, the pattern matching and PubKey extraction can be done via a single step, leading to the following simplified order with shorter code: 1) check if given script matches P2PK pattern with Solver (also contains valid PubKey length check), extracts Pubkey 2) create CPubKey object with extracted Pubkey 3) fully validate public key
2019-08-29Merge #16752: doc: Delete stale URL in test READMEfanquake
41d484d5c88f057e75cfdd8b88587b9a12a61744 doc: Delete stale URL in test README (Michael Folkson) Pull request description: The resource on the Boost unit test framework previously linked to in src/test/README.md was a stale URL. Instead of deleting it, I've replaced it with an alternative resource on the framework on [boost.org](https://www.boost.org/doc/libs/1_45_0/libs/test/doc/html/utf/tutorials.html). ACKs for top commit: promag: ACK 41d484d5c88f057e75cfdd8b88587b9a12a61744. hebasto: ACK 41d484d5c88f057e75cfdd8b88587b9a12a61744, the removed link is really obsolete. fanquake: ACK 41d484d5c88f057e75cfdd8b88587b9a12a61744 - Thanks. Tree-SHA512: 764f12548441bde615f77b7a2ca7c5188b4ab936972d16b84960fbd8604d4cbd224415bc59ce839e7e63293aa84fd97f31a69e38734e531231cdb0e148d2e1bd
2019-08-29Merge #16708: qt: Replace obsolete functions of QSslSocketWladimir J. van der Laan
2e1455c4a100170c322165af3ecbbcd3004b2f9d Replace obsolete functions of QSslSocket (Hennadii Stepanov) Pull request description: The [`QSslSocket::setDefaultCaCertificates()`](https://doc.qt.io/qt-5/qsslsocket-obsolete.html#setDefaultCaCertificates) and [`QSslSocket::systemCaCertificates()`](https://doc.qt.io/qt-5/qsslsocket-obsolete.html#systemCaCertificates) member functions are [obsolete](https://doc.qt.io/qt-5.12/qsslsocket-obsolete.html) since Qt 5.12. This PR replaces them, does not change behavior and keeps compatibility with [Qt 5.5.1](https://github.com/bitcoin/bitcoin/pull/15393). ACKs for top commit: laanwj: ACK 2e1455c4a100170c322165af3ecbbcd3004b2f9d promag: ACK 2e1455c4a100170c322165af3ecbbcd3004b2f9d. Tree-SHA512: 4182cd22a3e7a998d62a0fe84e748803a6962a65920b74da9fcf5666a700507468bb6e428054ccb70c2fbb7969a56933f450bc405c7a32ecbc1f8af4c1f983a3
2019-08-29doc: Delete stale URL in test READMEMichael Folkson
2019-08-28Merge #14879: qt: Add warning messages to the debug windowMarcoFalke
593ba696fb32da558091ac02ad87c4893db4ce97 Add warning messages to the debug window (Hennadii Stepanov) Pull request description: Fix: #11016 This PR adds warning messages to the debug window in `-disablewallet` mode. ![screenshot from 2018-12-06 01-01-27](https://user-images.githubusercontent.com/32963518/49550070-413c1c80-f8f3-11e8-9865-efb49ea8da45.png) ACKs for top commit: jonasschnelli: utACK 593ba696fb32da558091ac02ad87c4893db4ce97 promag: ACK 593ba696fb32da558091ac02ad87c4893db4ce97, agree with @Sjors https://github.com/bitcoin/bitcoin/pull/14879#pullrequestreview-196433092 above. ryanofsky: utACK 593ba696fb32da558091ac02ad87c4893db4ce97 Tree-SHA512: a8ca78529bb16813ba7bfaf5ccd4349189979f08e78ea857746a6fb00fd9d7ed98d8f06f384830acba21dac57070060af23f6be8249398feb32a6efff1333de8
2019-08-28wallet: Translate all initErrors in CreateWalletFromFileMarcoFalke
Every warning or error in this method is translated, except for those two. Translate them as well for consistency.
2019-08-28util: Make util/error bilingual_str (refactor)MarcoFalke
Translated strings should not end up in the debug log, stderr, or returned by an RPC. Changing the util methods in util/error to return a bilingual_str paves the way to achieve this goal in the long term.