💬 maaku commented on issue "macOS App Notarization":
(https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1865330437)
I'm a bit mystified that this issue is over 4 years old. Near as I can tell, the release manager just needs to pas `--hardened-runtime` to the signing command, and the resulting binaries will be notorizable. I've personally tested this on v26.
(https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1865330437)
I'm a bit mystified that this issue is over 4 years old. Near as I can tell, the release manager just needs to pas `--hardened-runtime` to the signing command, and the resulting binaries will be notorizable. I've personally tested this on v26.
💬 achow101 commented on issue "macOS App Notarization":
(https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1865370047)
> I'm a bit mystified that this issue is over 4 years old. Near as I can tell, the release manager just needs to pas `--hardened-runtime` to the signing command, and the resulting binaries will be notorizable. I've personally tested this on v26.
I don't think it's technically difficult, although it will probably need additional tooling and procedural changes to work in the Guix build system.
I believe the current main objection is a philosophical one to the fact that notarizing means that
...
(https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1865370047)
> I'm a bit mystified that this issue is over 4 years old. Near as I can tell, the release manager just needs to pas `--hardened-runtime` to the signing command, and the resulting binaries will be notorizable. I've personally tested this on v26.
I don't think it's technically difficult, although it will probably need additional tooling and procedural changes to work in the Guix build system.
I believe the current main objection is a philosophical one to the fact that notarizing means that
...
🤔 ryanofsky reviewed a pull request: "wallet: Have the wallet store the key for automatically generated descriptors"
(https://github.com/bitcoin/bitcoin/pull/26728#pullrequestreview-1791999003)
re: https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1865160784
re: https://github.com/bitcoin/bitcoin/pull/26728#pullrequestreview-1791595622
> This is essentially what we do today, albeit with hd keys rather than hd seeds.
Right. It seems like we can add the features we want without making the wallet data model more complicated, so I think we should do that. If there are features that can't work under the current model, or would work worse under the current model, that would c
...
(https://github.com/bitcoin/bitcoin/pull/26728#pullrequestreview-1791999003)
re: https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1865160784
re: https://github.com/bitcoin/bitcoin/pull/26728#pullrequestreview-1791595622
> This is essentially what we do today, albeit with hd keys rather than hd seeds.
Right. It seems like we can add the features we want without making the wallet data model more complicated, so I think we should do that. If there are features that can't work under the current model, or would work worse under the current model, that would c
...
💬 ryanofsky commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1433383580)
In commit "test: add coverage for re-opening a downgraded encrypted wallet on master" (98b6fad226640a0827e39574a91dd3d2fdeac94a)
I think your renaming script went too far 😁, this commit used to be authored by furszy (db6b61e9e7635c9cb97d73fd44b9e7266b8eef51)
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1433383580)
In commit "test: add coverage for re-opening a downgraded encrypted wallet on master" (98b6fad226640a0827e39574a91dd3d2fdeac94a)
I think your renaming script went too far 😁, this commit used to be authored by furszy (db6b61e9e7635c9cb97d73fd44b9e7266b8eef51)
💬 ryanofsky commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1433362520)
In commit "wallet: Automatically upgrade a wallet to have global hd key" (442962d4410edd985fe336f29070c55516ae7de4)
It seems like these variables are unnecessary, and CWallet::UpgradeToGlobalHDKey could just get the keys from the descriptors in the wallet instance, instead of requiring them to be passed as extra argument. This would require adding an extra loop to UpgradeToGlobalHDKey, but also make it less fragile, simpler to call, and easier to understand without having to look at outside c
...
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1433362520)
In commit "wallet: Automatically upgrade a wallet to have global hd key" (442962d4410edd985fe336f29070c55516ae7de4)
It seems like these variables are unnecessary, and CWallet::UpgradeToGlobalHDKey could just get the keys from the descriptors in the wallet instance, instead of requiring them to be passed as extra argument. This would require adding an extra loop to UpgradeToGlobalHDKey, but also make it less fragile, simpler to call, and easier to understand without having to look at outside c
...
💬 maaku commented on issue "macOS App Notarization":
(https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1865433661)
Not if the notarization is stapled to the binary, no. That should prevent phoning home. I have not used Wireshark to verify this, but that's what the documentation claims.
(https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1865433661)
Not if the notarization is stapled to the binary, no. That should prevent phoning home. I have not used Wireshark to verify this, but that's what the documentation claims.
💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1433433918)
Oops fixed
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1433433918)
Oops fixed
💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1865439673)
> Instead of returning an object with "xpub" and "xpriv" fields, it would return a list of objects with "xpub", "xpriv", and "descriptors" fields. The "descriptors" field would contain a list of descriptors using the hd key.
>
> It would also be good for the method to accept an "active_only" option, and only return hd keys associated with active descriptors by default.
>
> This would make it easy to get the hd key, and easy to understand how the hd key is referenced by descriptors. In the
...
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1865439673)
> Instead of returning an object with "xpub" and "xpriv" fields, it would return a list of objects with "xpub", "xpriv", and "descriptors" fields. The "descriptors" field would contain a list of descriptors using the hd key.
>
> It would also be good for the method to accept an "active_only" option, and only return hd keys associated with active descriptors by default.
>
> This would make it easy to get the hd key, and easy to understand how the hd key is referenced by descriptors. In the
...
💬 BrandonOdiwuor commented on pull request "getrawtransaction implementation":
(https://github.com/bitcoin-core/gui/pull/777#issuecomment-1865441605)
I have updated the PR based on the feedback given above:
- Added `Tools` menu
- Moved `getrawtransaction RPC` implementation to `Tools > verify external txid`
cc: @willcl-ark @luke-jr @pablomartin4btc
(https://github.com/bitcoin-core/gui/pull/777#issuecomment-1865441605)
I have updated the PR based on the feedback given above:
- Added `Tools` menu
- Moved `getrawtransaction RPC` implementation to `Tools > verify external txid`
cc: @willcl-ark @luke-jr @pablomartin4btc
📝 1wiz1 opened a pull request: "Rename SECURITY.md to SECURITY.md."
(https://github.com/bitcoin/bitcoin/pull/29128)
<!--
*** 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/29128)
<!--
*** 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
...
✅ achow101 closed a pull request: "Rename SECURITY.md to SECURITY.md."
(https://github.com/bitcoin/bitcoin/pull/29128)
(https://github.com/bitcoin/bitcoin/pull/29128)
📝 achow101 locked a pull request: "Rename SECURITY.md to SECURITY.md."
(https://github.com/bitcoin/bitcoin/pull/29128)
<!--
*** 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/29128)
<!--
*** 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
...
💬 LarryRuane commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#issuecomment-1865554603)
Yes, that's what the compiler error says, but that function doesn't seem to be in that namespace. Its other callers don't qualify it, and if I do qualify it, the build fails for me.
(https://github.com/bitcoin/bitcoin/pull/26564#issuecomment-1865554603)
Yes, that's what the compiler error says, but that function doesn't seem to be in that namespace. Its other callers don't qualify it, and if I do qualify it, the build fails for me.
💬 stratospher commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1433633597)
makes sense! added an option in `restart_node()` to restart the node with an empty addrman which could potentially be useful in other tests. addresses added in addpeeraddress test don't leak into addrman tests now.
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1433633597)
makes sense! added an option in `restart_node()` to restart the node with an empty addrman which could potentially be useful in other tests. addresses added in addpeeraddress test don't leak into addrman tests now.
💬 stratospher commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1433633671)
true, i've removed the comment in commit.
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1433633671)
true, i've removed the comment in commit.
💬 stratospher commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1433634609)
right, i've included both improvements!
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1433634609)
right, i've included both improvements!
💬 stratospher commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#issuecomment-1865704915)
thanks for the thoughtful feedback @amitiuttarwar, @0xB10C! I've updated the PR to address your comments.
Main changes were: ([git diff](https://github.com/bitcoin/bitcoin/compare/d3c24183483e43dd38209dbaf3c4ac725e13d4a3..1b49149b3ed7366141f0090692256e82032cb9af))
1. adding an option to restart the node with an empty addrman (method 1 suggested here https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1418564026) to prevent addpeeraddress test leaking into getaddrmaninfo and getrawaddr
...
(https://github.com/bitcoin/bitcoin/pull/29007#issuecomment-1865704915)
thanks for the thoughtful feedback @amitiuttarwar, @0xB10C! I've updated the PR to address your comments.
Main changes were: ([git diff](https://github.com/bitcoin/bitcoin/compare/d3c24183483e43dd38209dbaf3c4ac725e13d4a3..1b49149b3ed7366141f0090692256e82032cb9af))
1. adding an option to restart the node with an empty addrman (method 1 suggested here https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1418564026) to prevent addpeeraddress test leaking into getaddrmaninfo and getrawaddr
...
💬 jonasschnelli commented on issue "macOS App Notarization":
(https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1865735331)
@maaku
My tests in 2020 (https://github.com/bitcoin/bitcoin/pull/18187#issuecomment-592453829) showed me, that regardless of stapling, macOS does an onlinecheck of the notarization (if the user has internet connection).
Not sure if that has changed. Might be worth to redo https://github.com/bitcoin/bitcoin/pull/18187#issuecomment-592453829.
That unnecessary "calling-apple" was the reason to not do notarization back in 2020.
(https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1865735331)
@maaku
My tests in 2020 (https://github.com/bitcoin/bitcoin/pull/18187#issuecomment-592453829) showed me, that regardless of stapling, macOS does an onlinecheck of the notarization (if the user has internet connection).
Not sure if that has changed. Might be worth to redo https://github.com/bitcoin/bitcoin/pull/18187#issuecomment-592453829.
That unnecessary "calling-apple" was the reason to not do notarization back in 2020.
💬 maaku commented on issue "macOS App Notarization":
(https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1865811312)
@jonasschnelli Sounds like bug 57278824, which was fixed back in 2020 just a few months after you ran your test:
> There’s one further wrinkle here: Once Brigitte removed the com.apple.security.cs.disable-library-validation entitlement from their app the problem didn’t go away! And that’s the result of a historical bug in Gatekeeper (r. 57278824). Prior to 10.15.4 Gatekeeper does not recognise the implied library validation that you get when you enabled the hardened runtime. So, even though y
...
(https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1865811312)
@jonasschnelli Sounds like bug 57278824, which was fixed back in 2020 just a few months after you ran your test:
> There’s one further wrinkle here: Once Brigitte removed the com.apple.security.cs.disable-library-validation entitlement from their app the problem didn’t go away! And that’s the result of a historical bug in Gatekeeper (r. 57278824). Prior to 10.15.4 Gatekeeper does not recognise the implied library validation that you get when you enabled the hardened runtime. So, even though y
...
:lock: fanquake locked an issue: "Transfer from closed coinbase acc."
(https://github.com/bitcoin/bitcoin/issues/29126)
(https://github.com/bitcoin/bitcoin/issues/29126)