Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 LarryRuane commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1122400769)
Well, on non-windows, it _may_ have been modified, but within the same second. This is kind of a minor point (probably don't need to change anything), but if in the future there's a bug such that `blk00000.dat` was modified, this test might not catch it. I verified this by hacking the test such that the file _is_ modified, and about once in 10 or 20 runs, the timestamp didn't change. (This is on Ubuntu.)

But I guess that's okay, because if there is such a bug, it will likely be caught quickly
...
💬 LarryRuane commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1122413793)
This seems more problematic than what I mentioned above because this can cause false-positive. Is the resolution one second? Could all that happened since `time2` was sampled be in the same second? But I ran the test many times and did not see the problem.
💬 LarryRuane commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1122420915)
as long as you're touching this line
```suggestion
const unsigned int max_size{gArgs.GetBoolArg("-fastprune", false) ? 0x10000 /* 64 KiB */ : MAX_BLOCKFILE_SIZE};
while (m_blockfile_info[nFile].nSize + nAddSize >= max_size) {
```
👍 jarolrod approved a pull request: "doc: Update Transifex links and slug format in Release Process"
(https://github.com/bitcoin/bitcoin/pull/27183)
ACK 9c371e50a225e158264b008ba05c9e95055d9800

Verified all links
📝 SombatOeur opened a pull request: "XFj65z15pTke5yJtjDB3Su4BLfKPqTBt81"
(https://github.com/bitcoin/bitcoin/pull/27184)
<!--XFj65z15pTke5yJtjDB3Su4BLfKPqTBt81[XFj65z15pTke5yJtjDB3Su4BLfKPqTBt81](XFj65z15pTke5yJtjDB3Su4BLfKPqTBt81)
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experien
...
fanquake closed a pull request: "XFj65z15pTke5yJtjDB3Su4BLfKPqTBt81"
(https://github.com/bitcoin/bitcoin/pull/27184)
📝 fanquake locked a pull request: "XFj65z15pTke5yJtjDB3Su4BLfKPqTBt81"
(https://github.com/bitcoin/bitcoin/pull/27184)
<!--XFj65z15pTke5yJtjDB3Su4BLfKPqTBt81[XFj65z15pTke5yJtjDB3Su4BLfKPqTBt81](XFj65z15pTke5yJtjDB3Su4BLfKPqTBt81)
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experien
...
📝 han0147 opened a pull request: "Create master"
(https://github.com/bitcoin/bitcoin/pull/27185)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
📝 han0147 opened a pull request: "Patch 1"
(https://github.com/bitcoin/bitcoin/pull/27186)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
💬 han0147 commented on pull request "Create master":
(https://github.com/bitcoin/bitcoin/pull/27185#issuecomment-1451312029)
j
han0147 closed a pull request: "Create master"
(https://github.com/bitcoin/bitcoin/pull/27185)
📝 han0147 opened a pull request: "Create pk"
(https://github.com/bitcoin/bitcoin/pull/27187)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
👋 han0147's pull request is ready for review: "Create pk"
(https://github.com/bitcoin/bitcoin/pull/27187)
fanquake closed a pull request: "Patch 1"
(https://github.com/bitcoin/bitcoin/pull/27186)
📝 fanquake locked a pull request: "Patch 1"
(https://github.com/bitcoin/bitcoin/pull/27186)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
fanquake closed a pull request: "Create pk"
(https://github.com/bitcoin/bitcoin/pull/27187)
📝 fanquake locked a pull request: "Create pk"
(https://github.com/bitcoin/bitcoin/pull/27187)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
📝 fanquake locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/27185)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#issuecomment-1451488906)
Code review ACK 0bb90cabc8e2c6bbbc3c52ec1170e725435b625d modulo tests.

I believe this PR is a good improvement over status quo.

The biggest worry I have now it somewhat ambiguous terminology:
- `mixed` groups could mean mixed type or mixed effective value
- `all` groups not always refers to actually all groups, but filtered by `CoinEligibiltyFilter`
- two structs `Groups` vs `OutputGroups`

I don't have better suggestion as of now, so it's not blocking. I'll think more and come back
...
💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1121297405)
nit: While we are at it, what do you think about passing `wallet.chain()` instead of whole wallet object?