💬 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`).
💬 luke-jr commented on pull request "index: block filters sync, reduce disk read operations by caching last header":
(https://github.com/bitcoin/bitcoin/pull/28955#issuecomment-1839900641)
>Probably, it is because the benchmarks, same as the unit tests, create temp directories. Which may be faster to access than regular directories.
Is it, though? I would think the OS should have this cached regardless?
(https://github.com/bitcoin/bitcoin/pull/28955#issuecomment-1839900641)
>Probably, it is because the benchmarks, same as the unit tests, create temp directories. Which may be faster to access than regular directories.
Is it, though? I would think the OS should have this cached regardless?
🤔 luke-jr requested changes to a pull request: "build: disable external-signer for Windows"
(https://github.com/bitcoin/bitcoin/pull/28967#pullrequestreview-1763820931)
Let's at least let builders explicitly enable it?
(https://github.com/bitcoin/bitcoin/pull/28967#pullrequestreview-1763820931)
Let's at least let builders explicitly enable it?
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-1839920097)
@martinus thank you very much for your review! I updated the PR with all your suggestions.
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-1839920097)
@martinus thank you very much for your review! I updated the PR with all your suggestions.
💬 luke-jr commented on pull request "doc: explain what the wallet password does":
(https://github.com/bitcoin/bitcoin/pull/28974#discussion_r1414811800)
There is no seed phrase.
(https://github.com/bitcoin/bitcoin/pull/28974#discussion_r1414811800)
There is no seed phrase.
💬 luke-jr commented on pull request "doc: explain what the wallet password does":
(https://github.com/bitcoin/bitcoin/pull/28974#discussion_r1414812287)
Someone with such access can just install a keylogger.
(https://github.com/bitcoin/bitcoin/pull/28974#discussion_r1414812287)
Someone with such access can just install a keylogger.
💬 luke-jr commented on pull request "doc: explain what the wallet password does":
(https://github.com/bitcoin/bitcoin/pull/28974#discussion_r1414810714)
No need to say "introduced in" especially since v0.4.0 was not Bitcoin Core in any meaningful way
(https://github.com/bitcoin/bitcoin/pull/28974#discussion_r1414810714)
No need to say "introduced in" especially since v0.4.0 was not Bitcoin Core in any meaningful way