💬 jonatack commented on pull request "rpc: encryptwallet help, mention HD seed rotation and backup requirement":
(https://github.com/bitcoin/bitcoin/pull/28980#discussion_r1414581640)
> and blames core for not detecting the balance
I think I would avoid recommending that users delete their wallet files, as potential loss of funds is a worse case.
(https://github.com/bitcoin/bitcoin/pull/28980#discussion_r1414581640)
> and blames core for not detecting the balance
I think I would avoid recommending that users delete their wallet files, as potential loss of funds is a worse case.
💬 furszy commented on pull request "rpc: encryptwallet help, mention HD seed rotation and backup requirement":
(https://github.com/bitcoin/bitcoin/pull/28980#discussion_r1414582511)
> using the `backupwallet` RPC.
I think the current text is slightly better because it alerts users without scaring them.
But, if there is a higher preference for it, could change it.
(https://github.com/bitcoin/bitcoin/pull/28980#discussion_r1414582511)
> using the `backupwallet` RPC.
I think the current text is slightly better because it alerts users without scaring them.
But, if there is a higher preference for it, could change it.
💬 furszy commented on pull request "rpc: encryptwallet help, mention HD seed rotation and backup requirement":
(https://github.com/bitcoin/bitcoin/pull/28980#discussion_r1414585672)
Sure. Done as suggested
(https://github.com/bitcoin/bitcoin/pull/28980#discussion_r1414585672)
Sure. Done as suggested
💬 furszy commented on pull request "rpc: encryptwallet help, mention HD seed rotation and backup requirement":
(https://github.com/bitcoin/bitcoin/pull/28980#issuecomment-1839609896)
Updated per feedback. Thanks everyone.
(https://github.com/bitcoin/bitcoin/pull/28980#issuecomment-1839609896)
Updated per feedback. Thanks everyone.
🤔 ryanofsky reviewed a pull request: "doc: Add multiprocess design doc"
(https://github.com/bitcoin/bitcoin/pull/28978#pullrequestreview-1763577959)
Updated f566245147003648099f961306be82ea32ea47ae -> 837c53d14f24924cdcb2cfd8b18915882dc3b620 ([`pr/ipcdoc.7`](https://github.com/ryanofsky/bitcoin/commits/pr/ipcdoc.7) -> [`pr/ipcdoc.8`](https://github.com/ryanofsky/bitcoin/commits/pr/ipcdoc.8), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipcdoc.7..pr/ipcdoc.8)) adding suggested future enhancement and making a few other tweaks
(https://github.com/bitcoin/bitcoin/pull/28978#pullrequestreview-1763577959)
Updated f566245147003648099f961306be82ea32ea47ae -> 837c53d14f24924cdcb2cfd8b18915882dc3b620 ([`pr/ipcdoc.7`](https://github.com/ryanofsky/bitcoin/commits/pr/ipcdoc.7) -> [`pr/ipcdoc.8`](https://github.com/ryanofsky/bitcoin/commits/pr/ipcdoc.8), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipcdoc.7..pr/ipcdoc.8)) adding suggested future enhancement and making a few other tweaks
💬 ryanofsky commented on pull request "doc: Add multiprocess design doc":
(https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1414593514)
re: https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1413316233
> * Generating server / client code in different langages (e.g C++ -> Rust or Rust -> C++) from a common set of `capnp` files and with compatible interfaces.
Yes, this significant and worth mentioning. Added some information about this and a link the work you mentioned.
(https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1414593514)
re: https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1413316233
> * Generating server / client code in different langages (e.g C++ -> Rust or Rust -> C++) from a common set of `capnp` files and with compatible interfaces.
Yes, this significant and worth mentioning. Added some information about this and a link the work you mentioned.
💬 mzumsande commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1414612161)
This could lead to confusing log messages when removing 0-fee (or deprioritized) transactions: Such a tx could be refused due to "size limit" even if the mempool was almost empty.
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1414612161)
This could lead to confusing log messages when removing 0-fee (or deprioritized) transactions: Such a tx could be refused due to "size limit" even if the mempool was almost empty.
🤔 mzumsande reviewed a pull request: "v3 transaction policy for anti-pinning"
(https://github.com/bitcoin/bitcoin/pull/28948#pullrequestreview-1763610776)
> Keeping 0-fee parents hanging around in mempool would also allow new entries into the mempool that will never be mined.
Would that be possible with v3, and that's the reason why it's part of this PR? I can see the scenario where the bumping child spends another transaction's input and could get removed when that other tx gets replaced, leaving the 0-fee tx, but wouldn't that scenario be impossible with the v3 limit to one ancestor?
As a side effect, I think this will allow users to remov
...
(https://github.com/bitcoin/bitcoin/pull/28948#pullrequestreview-1763610776)
> Keeping 0-fee parents hanging around in mempool would also allow new entries into the mempool that will never be mined.
Would that be possible with v3, and that's the reason why it's part of this PR? I can see the scenario where the bumping child spends another transaction's input and could get removed when that other tx gets replaced, leaving the 0-fee tx, but wouldn't that scenario be impossible with the v3 limit to one ancestor?
As a side effect, I think this will allow users to remov
...
💬 ishaanam commented on pull request "wallet, rpc: add anti-fee-sniping to `send` and `sendall`":
(https://github.com/bitcoin/bitcoin/pull/28944#discussion_r1414651395)
Done
(https://github.com/bitcoin/bitcoin/pull/28944#discussion_r1414651395)
Done
💬 ishaanam commented on pull request "wallet, rpc: add anti-fee-sniping to `send` and `sendall`":
(https://github.com/bitcoin/bitcoin/pull/28944#discussion_r1414651744)
Done
(https://github.com/bitcoin/bitcoin/pull/28944#discussion_r1414651744)
Done
💬 ishaanam commented on pull request "wallet, rpc: add anti-fee-sniping to `send` and `sendall`":
(https://github.com/bitcoin/bitcoin/pull/28944#discussion_r1414652209)
Done
(https://github.com/bitcoin/bitcoin/pull/28944#discussion_r1414652209)
Done
💬 luke-jr commented on pull request "Fix SSE4.1-related issues":
(https://github.com/bitcoin/bitcoin/pull/28893#issuecomment-1839830013)
Weak Concept NACK: We should probably still build the SSE4.1 code if the compiler supports it. The flag should only affect the rest of the codebase, so that the software will run on a system without SSE4.1 (but still use the optimised crypto on systems that have it).
(https://github.com/bitcoin/bitcoin/pull/28893#issuecomment-1839830013)
Weak Concept NACK: We should probably still build the SSE4.1 code if the compiler supports it. The flag should only affect the rest of the codebase, so that the software will run on a system without SSE4.1 (but still use the optimised crypto on systems that have it).
💬 luke-jr commented on issue "build: Configuring with `-mno-sse4.1` does not fail the sse4.1 instrinsics check":
(https://github.com/bitcoin/bitcoin/issues/28864#issuecomment-1839831023)
This is fixed by #13789 which was inexplicably closed for no reason
(https://github.com/bitcoin/bitcoin/issues/28864#issuecomment-1839831023)
This is fixed by #13789 which was inexplicably closed for no reason
💬 luke-jr commented on pull request "wallet: birth time update during tx scanning":
(https://github.com/bitcoin/bitcoin/pull/28920#discussion_r1414735112)
Shouldn't this be `NO_KEY_TIME` explicitly?
(https://github.com/bitcoin/bitcoin/pull/28920#discussion_r1414735112)
Shouldn't this be `NO_KEY_TIME` explicitly?
💬 luke-jr commented on pull request "wallet: birth time update during tx scanning":
(https://github.com/bitcoin/bitcoin/pull/28920#discussion_r1414735331)
Seems like `NO_KEY_TIME` ought to be `0` so it triggers a full scan
(https://github.com/bitcoin/bitcoin/pull/28920#discussion_r1414735331)
Seems like `NO_KEY_TIME` ought to be `0` so it triggers a full scan
💬 luke-jr commented on pull request "script/sign: avoid duplicated signature verification after signing (+introduce signing benchmarks)":
(https://github.com/bitcoin/bitcoin/pull/28923#issuecomment-1839848382)
Does this reduce our safety against memory corruption or similar?
(https://github.com/bitcoin/bitcoin/pull/28923#issuecomment-1839848382)
Does this reduce our safety against memory corruption or similar?
💬 luke-jr commented on pull request "wallet: Add scan_utxo option to getbalances RPC":
(https://github.com/bitcoin/bitcoin/pull/28930#issuecomment-1839854997)
getbalances seems like the wrong place for this?
(https://github.com/bitcoin/bitcoin/pull/28930#issuecomment-1839854997)
getbalances seems like the wrong place for this?
💬 techy2 commented on issue "getrawtransaction xxxxxx.... 2 causes a segfault":
(https://github.com/bitcoin/bitcoin/issues/28986#issuecomment-1839879785)
computer is an HP Proliant GL 380 G7 that has been up for over a year, it is very reliable and runs many other tasks. Don't think it is a hardware issue or I would think there would be other problems as well and there are not.
-tindex is off
The transaction is not one in my wallet, just a random tx from the network I picked off the block explorer, I was doing some testing and picked it just to have a look at the transaction structure as reported by the daemon. In hindsight, because it was
...
(https://github.com/bitcoin/bitcoin/issues/28986#issuecomment-1839879785)
computer is an HP Proliant GL 380 G7 that has been up for over a year, it is very reliable and runs many other tasks. Don't think it is a hardware issue or I would think there would be other problems as well and there are not.
-tindex is off
The transaction is not one in my wallet, just a random tx from the network I picked off the block explorer, I was doing some testing and picked it just to have a look at the transaction structure as reported by the daemon. In hindsight, because it was
...
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1414758537)
Only 2 of the 6 previous `flags` modifications used copy assignment, the other 4 use OR assignment. The 2 that use copy assignment we know logically that they were just created and therefore their flags are 0 and we are not overwriting anything. Therefore an OR assignment in those 2 cases is logically equivalent.
So if we were to go with your second suggestion we would need to modify behavior to make sure we assign the new flags we wish to set OR'd with the current flags. I think that would mak
...
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1414758537)
Only 2 of the 6 previous `flags` modifications used copy assignment, the other 4 use OR assignment. The 2 that use copy assignment we know logically that they were just created and therefore their flags are 0 and we are not overwriting anything. Therefore an OR assignment in those 2 cases is logically equivalent.
So if we were to go with your second suggestion we would need to modify behavior to make sure we assign the new flags we wish to set OR'd with the current flags. I think that would mak
...
🤔 pablomartin4btc reviewed a pull request: "Fee Estimator updates from Validation Interface/CScheduler thread"
(https://github.com/bitcoin/bitcoin/pull/28368#pullrequestreview-1763789905)
post merge tACK 91504cbe0de2b74ef1aa2709761aaf0597ec66a2
Tested manually using `RPC` commands `estimatesmartfee` and `estimaterawfee` (following cases from `/test/functional/feature_fee_estimation.py`).
(https://github.com/bitcoin/bitcoin/pull/28368#pullrequestreview-1763789905)
post merge tACK 91504cbe0de2b74ef1aa2709761aaf0597ec66a2
Tested manually using `RPC` commands `estimatesmartfee` and `estimaterawfee` (following cases from `/test/functional/feature_fee_estimation.py`).