π¬ adamandrews1 commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2172923841)
Thanks, I will update this comment
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2172923841)
Thanks, I will update this comment
π¬ tnndbtc commented on issue "bitcoind shouldn't fail to progress with synchronization: endless [leveldb] Generated table ... logs":
(https://github.com/bitcoin/bitcoin/issues/31882#issuecomment-3014488356)
@GregTonoski If it's hard to profile the bitcoind process, and if you can still reproduce the issue, could you take several pstacks, each with 1 minute apart, and see which function appears multiple times? Then those are the potential ones being stuck. You can upload the pstacks so I can help take a look.
(https://github.com/bitcoin/bitcoin/issues/31882#issuecomment-3014488356)
@GregTonoski If it's hard to profile the bitcoind process, and if you can still reproduce the issue, could you take several pstacks, each with 1 minute apart, and see which function appears multiple times? Then those are the potential ones being stuck. You can upload the pstacks so I can help take a look.
π¬ murchandamus commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2172976692)
I was just rereading the commit "wallet: Change balance calculation to use m_txos" (dde7cbe105ba6daaa636466d0fa3a83d15609417) and Iβm aware that this is not a behavior change, but I am wondering about how the balances are calculated.
As seen, this distinguishes three groups of TXOs:
- "immature"
`if (wallet.IsTxImmatureCoinBase(wtx) && wtx.isConfirmed()) {`
- "trusted"
`} else if (is_trusted && tx_depth >= min_depth) {`
- "untrusted_pending"
`} else if (!is_trusted && wtx.InMemp
...
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2172976692)
I was just rereading the commit "wallet: Change balance calculation to use m_txos" (dde7cbe105ba6daaa636466d0fa3a83d15609417) and Iβm aware that this is not a behavior change, but I am wondering about how the balances are calculated.
As seen, this distinguishes three groups of TXOs:
- "immature"
`if (wallet.IsTxImmatureCoinBase(wtx) && wtx.isConfirmed()) {`
- "trusted"
`} else if (is_trusted && tx_depth >= min_depth) {`
- "untrusted_pending"
`} else if (!is_trusted && wtx.InMemp
...
π€ murchandamus reviewed a pull request: "wallet: Keep track of the wallet's own transaction outputs in memory"
(https://github.com/bitcoin/bitcoin/pull/27286#pullrequestreview-2968182392)
ACK 215e5999e20
(https://github.com/bitcoin/bitcoin/pull/27286#pullrequestreview-2968182392)
ACK 215e5999e20
π volcano89 opened a pull request: "#WΔ°NDOWS11"
(https://github.com/bitcoin/bitcoin/pull/32824)
<!--
*** 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
...
(https://github.com/bitcoin/bitcoin/pull/32824)
<!--
*** 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
...
π¬ volcano89 commented on pull request "#WΔ°NDOWS11":
(https://github.com/bitcoin/bitcoin/pull/32824#issuecomment-3014904744)
### ### ****
(https://github.com/bitcoin/bitcoin/pull/32824#issuecomment-3014904744)
### ### ****
π¬ pythcoiner commented on pull request "descriptors: Allow `H` as a hardened indicator":
(https://github.com/bitcoin/bitcoin/pull/32788#issuecomment-3014915905)
> Is anyone using it?
support for `H` have been introduced in embit in [this commit](https://github.com/diybitcoinhardware/embit/pull/47/commits/1696d6815cf6c2ddb7f56dfbf3bc50162e463647) but afair I add `H` because it was in the BIP not because it was needed.
(https://github.com/bitcoin/bitcoin/pull/32788#issuecomment-3014915905)
> Is anyone using it?
support for `H` have been introduced in embit in [this commit](https://github.com/diybitcoinhardware/embit/pull/47/commits/1696d6815cf6c2ddb7f56dfbf3bc50162e463647) but afair I add `H` because it was in the BIP not because it was needed.
π¬ volcano89 commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-3014916497)
### ###
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-3014916497)
### ###
π¬ volcano89 commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-3014916594)
https://gist.github.com/volcano89
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-3014916594)
https://gist.github.com/volcano89
π¬ romanz commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2173142997)
Thanks!
Will open a new PR to fix.
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2173142997)
Thanks!
Will open a new PR to fix.
π¬ romanz commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#issuecomment-3015014284)
Many thanks!
(https://github.com/bitcoin/bitcoin/pull/32540#issuecomment-3015014284)
Many thanks!
π romanz opened a pull request: "rest: rename `strURIPart` -> `uri_part`"
(https://github.com/bitcoin/bitcoin/pull/32825)
Following https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2172902737.
<!--
*** 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 devel
...
(https://github.com/bitcoin/bitcoin/pull/32825)
Following https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2172902737.
<!--
*** 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 devel
...
π¬ romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3015029219)
Rebased over `master` and fixed `rest_tx_from_block()` argument name case (in addition to https://github.com/bitcoin/bitcoin/pull/32825).
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3015029219)
Rebased over `master` and fixed `rest_tx_from_block()` argument name case (in addition to https://github.com/bitcoin/bitcoin/pull/32825).
π¬ hodlinator commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2173159314)
True, sole change in latest push.
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2173159314)
True, sole change in latest push.
π¬ polespinasa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#issuecomment-3015120652)
db63d5bf81 reverts to previous status (do not review). Will apply the commented changes in the next push.
(https://github.com/bitcoin/bitcoin/pull/32750#issuecomment-3015120652)
db63d5bf81 reverts to previous status (do not review). Will apply the commented changes in the next push.
π¬ maflcko commented on pull request "rest: rename `strURIPart` -> `uri_part`":
(https://github.com/bitcoin/bitcoin/pull/32825#issuecomment-3015151022)
could be a scripted-diff? See e.g. 8888beea8d477b1d4a2dfd2a0bb5f686de62f3ff
(https://github.com/bitcoin/bitcoin/pull/32825#issuecomment-3015151022)
could be a scripted-diff? See e.g. 8888beea8d477b1d4a2dfd2a0bb5f686de62f3ff
π¬ purpleKarrot commented on pull request "cmake: Introduce `WITH_PYTHON` build option":
(https://github.com/bitcoin/bitcoin/pull/31669#issuecomment-3015173238)
NACK
I am with @fanquake here. The complexity increases exponentially with the number of build options.
Since the proposed build option does not actually affect build artifacts (like optional python bindings for some library would do), this does not really require a user facing setting. The approach in https://github.com/bitcoin/bitcoin/pull/31233 is much more elegant.
(https://github.com/bitcoin/bitcoin/pull/31669#issuecomment-3015173238)
NACK
I am with @fanquake here. The complexity increases exponentially with the number of build options.
Since the proposed build option does not actually affect build artifacts (like optional python bindings for some library would do), this does not really require a user facing setting. The approach in https://github.com/bitcoin/bitcoin/pull/31233 is much more elegant.
π¬ musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#issuecomment-3015180485)
> Concept ACK [be75fa4](https://github.com/bitcoin/bitcoin/commit/be75fa48fa77c417b9ebdd3be4d1ddb212ab16b3)
>
> Might be good to add release notes for this PR as it is adding to the user-facing RPC interface, similarly to: https://github.com/bitcoin/bitcoin/blob/be75fa48fa77c417b9ebdd3be4d1ddb212ab16b3/doc/release-notes-27826.md
Added in [ede20b4](https://github.com/bitcoin/bitcoin/commit/ede20b438cf62b8292a3cae95893d34c3faa5b0c)
(https://github.com/bitcoin/bitcoin/pull/32800#issuecomment-3015180485)
> Concept ACK [be75fa4](https://github.com/bitcoin/bitcoin/commit/be75fa48fa77c417b9ebdd3be4d1ddb212ab16b3)
>
> Might be good to add release notes for this PR as it is adding to the user-facing RPC interface, similarly to: https://github.com/bitcoin/bitcoin/blob/be75fa48fa77c417b9ebdd3be4d1ddb212ab16b3/doc/release-notes-27826.md
Added in [ede20b4](https://github.com/bitcoin/bitcoin/commit/ede20b438cf62b8292a3cae95893d34c3faa5b0c)
π¬ musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2173209518)
Fixed in [b5d472](https://github.com/bitcoin/bitcoin/pull/32800/commits/b5d472e1c5436a9398c533d7ff07767d13399a5b). Thanks!
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2173209518)
Fixed in [b5d472](https://github.com/bitcoin/bitcoin/pull/32800/commits/b5d472e1c5436a9398c533d7ff07767d13399a5b). Thanks!