💬 ismaelsadeeq commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1295134019)
nit:
I know you describe why the `max_size` is `1024` in commit message.
But will it be nice if you add a comment about the `1024` / `1023` `max_size`.
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1295134019)
nit:
I know you describe why the `max_size` is `1024` in commit message.
But will it be nice if you add a comment about the `1024` / `1023` `max_size`.
👍 theStack approved a pull request: "crypto: more `Span<std::byte>` modernization & follow-ups"
(https://github.com/bitcoin/bitcoin/pull/28100#pullrequestreview-1579627219)
Code-review ACK baf93fbd47058b182643c5a11fe9a8928276b05f
Potential follow-up nit: there might be some more instances where using `BOOST_CHECK_EQUAL` over `BOOST_CHECK` could make sense, see `git grep "BOOST_CHECK(.*==" ./src/test/crypto_tests.cpp`.
(https://github.com/bitcoin/bitcoin/pull/28100#pullrequestreview-1579627219)
Code-review ACK baf93fbd47058b182643c5a11fe9a8928276b05f
Potential follow-up nit: there might be some more instances where using `BOOST_CHECK_EQUAL` over `BOOST_CHECK` could make sense, see `git grep "BOOST_CHECK(.*==" ./src/test/crypto_tests.cpp`.
💬 russeree commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1679799885)
@Retropex The statement provided is fairly rife with fallacies. Noting a couple of them here with a solution at the bottom.
> Although there was a debate in particular with the BIP47, we must not forget that some people like Mike are there only to protect their business with the src-20 and stamps. They absolutely do not care about the decentralization of Bitcoin and the consequences it has on the nodes.
Ad Hominem (Attacking the Person): Just because Mike has a business interest doesn't au
...
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1679799885)
@Retropex The statement provided is fairly rife with fallacies. Noting a couple of them here with a solution at the bottom.
> Although there was a debate in particular with the BIP47, we must not forget that some people like Mike are there only to protect their business with the src-20 and stamps. They absolutely do not care about the decentralization of Bitcoin and the consequences it has on the nodes.
Ad Hominem (Attacking the Person): Just because Mike has a business interest doesn't au
...
💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1295294960)
Done!
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1295294960)
Done!
💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1295294996)
Done!
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1295294996)
Done!
💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1295295485)
Done!
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1295295485)
Done!
💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#issuecomment-1679841564)
Force pushed addressing https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1293959610, https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1293962271, https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1293962878 and changed the `cost_of_change` according to `spend.cpp`, it avoids any issue with low targets.
(https://github.com/bitcoin/bitcoin/pull/27585#issuecomment-1679841564)
Force pushed addressing https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1293959610, https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1293962271, https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1293962878 and changed the `cost_of_change` according to `spend.cpp`, it avoids any issue with low targets.
💬 sipa commented on issue "Raise maximum -dbcache setting":
(https://github.com/bitcoin/bitcoin/issues/28249#issuecomment-1679918327)
@Sjors When doing a full IBD from genesis with dbcache set to 16 GiB, is the limit ever reached (or gotten close to)? If not, it's probably unnecessary to raise it.
(https://github.com/bitcoin/bitcoin/issues/28249#issuecomment-1679918327)
@Sjors When doing a full IBD from genesis with dbcache set to 16 GiB, is the limit ever reached (or gotten close to)? If not, it's probably unnecessary to raise it.
💬 amitiuttarwar commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/27511#issuecomment-1679933308)
ACK 6d7fd9fb9b38be75b2bce643a99cc584837a902b
(https://github.com/bitcoin/bitcoin/pull/27511#issuecomment-1679933308)
ACK 6d7fd9fb9b38be75b2bce643a99cc584837a902b
💬 MarcoFalke commented on pull request "ci: Fix macOS-cross SDK rsync":
(https://github.com/bitcoin/bitcoin/pull/28273#issuecomment-1679985361)
CI is green 🫠
(https://github.com/bitcoin/bitcoin/pull/28273#issuecomment-1679985361)
CI is green 🫠
💬 MarcoFalke commented on pull request "ci: Fix macOS-cross SDK rsync":
(https://github.com/bitcoin/bitcoin/pull/28273#discussion_r1295394612)
review note: This was done to manually nuke the old cache and start a new depends build/cache for all tasks on Cirrus CI.
(https://github.com/bitcoin/bitcoin/pull/28273#discussion_r1295394612)
review note: This was done to manually nuke the old cache and start a new depends build/cache for all tasks on Cirrus CI.
🤔 atou69 reviewed a pull request: "refactor: Remove unused boost signals2 from torcontrol"
(https://github.com/bitcoin/bitcoin/pull/28240#pullrequestreview-1579843842)
#27511
(https://github.com/bitcoin/bitcoin/pull/28240#pullrequestreview-1579843842)
#27511
💬 MarcoFalke commented on pull request "ci: label docker images and prune dangling images selectively":
(https://github.com/bitcoin/bitcoin/pull/27793#discussion_r1295419773)
this isn't enough. Volumes also exist
(https://github.com/bitcoin/bitcoin/pull/27793#discussion_r1295419773)
this isn't enough. Volumes also exist
⚠️ MarcoFalke opened an issue: "What is depends BUILD_ID_SALT ?"
(https://github.com/bitcoin/bitcoin/issues/28276)
Is it unused? If not, how can it be used? If yes, can it be removed?
References:
```
$ git grep '\<build_id\>'
depends/Makefile:# they rely on the build_id variables
depends/Makefile:build_id:=$(shell env CC='$(build_CC)' C_STANDARD='$(C_STANDARD)' CXX='$(build_CXX)' CXX_STANDARD='$(CXX_STANDARD)' AR='$(build_AR)' RANLIB='$(build_RANLIB)' STRIP='$(build_STRIP)' SHA256SUM='$(build_SHA256SUM)' DEBUG='$(DEBUG)' LTO='$(LTO)' NO_HARDEN='$(NO_HARDEN)' ./gen_id '$(BUILD_ID_SALT)' 'GUIX_
...
(https://github.com/bitcoin/bitcoin/issues/28276)
Is it unused? If not, how can it be used? If yes, can it be removed?
References:
```
$ git grep '\<build_id\>'
depends/Makefile:# they rely on the build_id variables
depends/Makefile:build_id:=$(shell env CC='$(build_CC)' C_STANDARD='$(C_STANDARD)' CXX='$(build_CXX)' CXX_STANDARD='$(CXX_STANDARD)' AR='$(build_AR)' RANLIB='$(build_RANLIB)' STRIP='$(build_STRIP)' SHA256SUM='$(build_SHA256SUM)' DEBUG='$(DEBUG)' LTO='$(LTO)' NO_HARDEN='$(NO_HARDEN)' ./gen_id '$(BUILD_ID_SALT)' 'GUIX_
...
💬 MarcoFalke commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1295448801)
Any reason to reduce this to 4 when it previously was 10?
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1295448801)
Any reason to reduce this to 4 when it previously was 10?
💬 MarcoFalke commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1295459732)
Looks like each pull request will rewrite the ccache and thus the hit rate will be ~0% overall?
Would be better to get the cache from the own pull only, or master.
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1295459732)
Looks like each pull request will rewrite the ccache and thus the hit rate will be ~0% overall?
Would be better to get the cache from the own pull only, or master.
💬 MarcoFalke commented on pull request "ci: Run "macOS 11.0 [gui, no tests] [jammy]" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28265#discussion_r1295494329)
> > Also, is the 10 GB enough to store all ccache + depends + image + ... stuff for all tasks?
>
> Well, it depends on what "all tasks" encompass.
A single task currently will cycle through 10GB in less than a day, see https://github.com/bitcoin/bitcoin/actions/caches
(https://github.com/bitcoin/bitcoin/pull/28265#discussion_r1295494329)
> > Also, is the 10 GB enough to store all ccache + depends + image + ... stuff for all tasks?
>
> Well, it depends on what "all tasks" encompass.
A single task currently will cycle through 10GB in less than a day, see https://github.com/bitcoin/bitcoin/actions/caches
💬 RobinLinus commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1680138110)
@russeree, your response is a red herring because you're distracting from the fact that Mike openly states that he's *"polluting the UTXO set"* with *"toxic waste"*:
- ["What if we do it in the worst way possible? What if we store it in the UTXO set?"](https://youtu.be/jJV_-EFZshU?t=252)
- ["I'm ruining bitcoin"](https://youtu.be/jJV_-EFZshU?t=276)
- ["With Stamps we're polluting the UTXO set on pre-segwit nodes, post-segwit nodes, pruned nodes, full nodes. They're all getting this toxic wa
...
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1680138110)
@russeree, your response is a red herring because you're distracting from the fact that Mike openly states that he's *"polluting the UTXO set"* with *"toxic waste"*:
- ["What if we do it in the worst way possible? What if we store it in the UTXO set?"](https://youtu.be/jJV_-EFZshU?t=252)
- ["I'm ruining bitcoin"](https://youtu.be/jJV_-EFZshU?t=276)
- ["With Stamps we're polluting the UTXO set on pre-segwit nodes, post-segwit nodes, pruned nodes, full nodes. They're all getting this toxic wa
...
💬 hebasto commented on pull request "ci: Fix macOS-cross SDK rsync":
(https://github.com/bitcoin/bitcoin/pull/28273#discussion_r1295521264)
Why is there a need to build a new `depends/built` cache in the first place?
(https://github.com/bitcoin/bitcoin/pull/28273#discussion_r1295521264)
Why is there a need to build a new `depends/built` cache in the first place?
💬 hebasto commented on pull request "ci: Fix macOS-cross SDK rsync":
(https://github.com/bitcoin/bitcoin/pull/28273#issuecomment-1680149151)
Re-run of the Cirrus task builds the depends packages regardless of caching: https://cirrus-ci.com/task/5981899345100800
(https://github.com/bitcoin/bitcoin/pull/28273#issuecomment-1680149151)
Re-run of the Cirrus task builds the depends packages regardless of caching: https://cirrus-ci.com/task/5981899345100800