💬 TheCharlatan commented on pull request "rpc, util: deduplicate AmountFromValue() using util::Result":
(https://github.com/bitcoin/bitcoin/pull/28134#discussion_r1272645850)
Including `rpc/` anything seems unfortunate to do here. Maybe these UniValue-specific utilities should instead be moved to their own files?
(https://github.com/bitcoin/bitcoin/pull/28134#discussion_r1272645850)
Including `rpc/` anything seems unfortunate to do here. Maybe these UniValue-specific utilities should instead be moved to their own files?
🤔 mzumsande reviewed a pull request: "Rework validation logic for assumeutxo"
(https://github.com/bitcoin/bitcoin/pull/27746#pullrequestreview-1544102750)
Code Review ACK 137762f34a845e491b80f9cea07efc4427cb38bf
I reviewed all commits again and did a bit of testing with snapshots on signet.
(https://github.com/bitcoin/bitcoin/pull/27746#pullrequestreview-1544102750)
Code Review ACK 137762f34a845e491b80f9cea07efc4427cb38bf
I reviewed all commits again and did a bit of testing with snapshots on signet.
💬 darosior commented on pull request "fuzz: Test headers pre-sync through p2p interface":
(https://github.com/bitcoin/bitcoin/pull/28043#discussion_r1272659160)
Why not disabling it completely instead of mocking it?
(https://github.com/bitcoin/bitcoin/pull/28043#discussion_r1272659160)
Why not disabling it completely instead of mocking it?
👍 theuni approved a pull request: "net processing, refactor: Decouple PeerManager from gArgs"
(https://github.com/bitcoin/bitcoin/pull/27499#pullrequestreview-1544114932)
Concept ACK and quick review ACK.
My only nit is that `size_t max_extra_txs` is platform dependent and says nothing about its upper bound. Similarly, from a quick glance, I think `max_orphan_txs` could wrap from the command line?
(I realize these aren't new problems, they're just highlighted here)
Maybe as a followup we could add some pedantic checks in init to make sure they're sane values that fit into 32bits and make `max_orphan_txs` a `uint32_t`. Please ignore if those checks exist
...
(https://github.com/bitcoin/bitcoin/pull/27499#pullrequestreview-1544114932)
Concept ACK and quick review ACK.
My only nit is that `size_t max_extra_txs` is platform dependent and says nothing about its upper bound. Similarly, from a quick glance, I think `max_orphan_txs` could wrap from the command line?
(I realize these aren't new problems, they're just highlighted here)
Maybe as a followup we could add some pedantic checks in init to make sure they're sane values that fit into 32bits and make `max_orphan_txs` a `uint32_t`. Please ignore if those checks exist
...
💬 theuni commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1272674822)
I like this, thanks! It just looks like a more modern version of the other functions. I also like that the weird no-value case is handled at the JSON layer instead. That's indeed cleaner than what I suggested.
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1272674822)
I like this, thanks! It just looks like a more modern version of the other functions. I also like that the weird no-value case is handled at the JSON layer instead. That's indeed cleaner than what I suggested.
📝 donCallm opened a pull request: "[TASK 0] FIRST CHANGE IN MY BLOCKCHAIN"
(https://github.com/bitcoin/bitcoin/pull/28140)
<!--
*** 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/28140)
<!--
*** 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: "[TASK 0] FIRST CHANGE IN MY BLOCKCHAIN"
(https://github.com/bitcoin/bitcoin/pull/28140)
(https://github.com/bitcoin/bitcoin/pull/28140)
📝 fanquake locked a pull request: "[TASK 0] FIRST CHANGE IN MY BLOCKCHAIN"
(https://github.com/bitcoin/bitcoin/pull/28140)
<!--
*** 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/28140)
<!--
*** 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
...
📝 donCallm opened a pull request: "[TASK 0] FIRST CHANGE IN MY BLOCKCHAIN"
(https://github.com/bitcoin/bitcoin/pull/28141)
<!--
*** 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/28141)
<!--
*** 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
...
✅ pinheadmz closed a pull request: "[TASK 0] FIRST CHANGE IN MY BLOCKCHAIN"
(https://github.com/bitcoin/bitcoin/pull/28141)
(https://github.com/bitcoin/bitcoin/pull/28141)
📝 fanquake locked a pull request: "[TASK 0] FIRST CHANGE IN MY BLOCKCHAIN"
(https://github.com/bitcoin/bitcoin/pull/28141)
<!--
*** 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/28141)
<!--
*** 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
...
👍 theuni approved a pull request: "kernel: Remove UniValue from kernel library"
(https://github.com/bitcoin/bitcoin/pull/28113#pullrequestreview-1544154683)
ACK 42233bea28f6f7c464f0cd6499d675e81b3e8512
Assuming CI is happy.
Very nice removal :)
(https://github.com/bitcoin/bitcoin/pull/28113#pullrequestreview-1544154683)
ACK 42233bea28f6f7c464f0cd6499d675e81b3e8512
Assuming CI is happy.
Very nice removal :)
💬 theuni commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1272685574)
Woohoo!
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1272685574)
Woohoo!
💬 sdaftuar commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1272692040)
See @achow's explanation: https://github.com/bitcoin/bitcoin/pull/27746/files#r1272483470
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1272692040)
See @achow's explanation: https://github.com/bitcoin/bitcoin/pull/27746/files#r1272483470
💬 manfreddd commented on issue "Bitcoin Core v25.0 Crashes":
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1648523348)
I ran the wmic command "diskdrive get status" and came back with status "Ok". I tried to run the CLI command "CHKDSK" but got message that disk was locked by another application. I reinitiated the computer and that included a disk repair job that ended with 100% completion. Also, I ran disk defragmentation, which hadn't been done in a while and took forever to complete.
I did the above to try and eliminate or minimize the incidence of any disk issues in future Bitcoin Core runs. I started Bit
...
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1648523348)
I ran the wmic command "diskdrive get status" and came back with status "Ok". I tried to run the CLI command "CHKDSK" but got message that disk was locked by another application. I reinitiated the computer and that included a disk repair job that ended with 100% completion. Also, I ran disk defragmentation, which hadn't been done in a while and took forever to complete.
I did the above to try and eliminate or minimize the incidence of any disk issues in future Bitcoin Core runs. I started Bit
...
💬 hebasto commented on issue "ci: Future of macOS and Windows MSVC CI tasks":
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1648548618)
Another consideration. AppVeyor still [has](https://www.appveyor.com/docs/build-configuration/#time-limitations):
> 60 minutes quota per build job.
That makes running functional tests questionable.
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1648548618)
Another consideration. AppVeyor still [has](https://www.appveyor.com/docs/build-configuration/#time-limitations):
> 60 minutes quota per build job.
That makes running functional tests questionable.
💬 sdaftuar commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#issuecomment-1648555652)
Thanks all for the additional review. I pushed a branch that I think addresses all of @ryanofsky's latest feedback (previous branch is tagged [here](https://github.com/sdaftuar/bitcoin/commits/27746.3) for reference).
(https://github.com/bitcoin/bitcoin/pull/27746#issuecomment-1648555652)
Thanks all for the additional review. I pushed a branch that I think addresses all of @ryanofsky's latest feedback (previous branch is tagged [here](https://github.com/sdaftuar/bitcoin/commits/27746.3) for reference).
💬 sdaftuar commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1272722353)
Done.
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1272722353)
Done.
💬 Kenshiro-28 commented on issue "Implement PayJoin / Pay-to-EndPoint":
(https://github.com/bitcoin/bitcoin/issues/19148#issuecomment-1648619222)
Optional payments with Payjoin would be great, even if it requires both parties are online.
(https://github.com/bitcoin/bitcoin/issues/19148#issuecomment-1648619222)
Optional payments with Payjoin would be great, even if it requires both parties are online.
📝 achow101 opened a pull request: "wallet: Allow users to create a wallet that encrypts all database records"
(https://github.com/bitcoin/bitcoin/pull/28142)
A couple of users have requested that we support wallets that encrypt everything, even if the wallet is watch-only, in order to have better privacy if the wallet is stolen. This PR introduces an `EncryptedDatabase` backend for the wallet which encrypts/decrypts each key-value record individually before reading from or writing to the database.
`EncryptedDatabase` is only supported for SQLite databases and descriptor wallets. This was done in order to have an easier way to get downgrade protect
...
(https://github.com/bitcoin/bitcoin/pull/28142)
A couple of users have requested that we support wallets that encrypt everything, even if the wallet is watch-only, in order to have better privacy if the wallet is stolen. This PR introduces an `EncryptedDatabase` backend for the wallet which encrypts/decrypts each key-value record individually before reading from or writing to the database.
`EncryptedDatabase` is only supported for SQLite databases and descriptor wallets. This was done in order to have an easier way to get downgrade protect
...