💬 luke-jr commented on pull request "CONTRIBUTING: Caution against using AI/LLMs (ChatGPT, Copilot, etc)":
(https://github.com/bitcoin/bitcoin/pull/28175#discussion_r1278081971)
I mentioned these specifically because:
1. ChatGPT is the most popularly known, and most likely to be searched for if someone is considering using it.
2. GitHub promotes use of Copilot heavily, and we are using GitHub.
3. Meta is falsely advertising LLaMA as open source, and many people are just believing that without verifying. (The source code is not available, and the license is not permissive)
(https://github.com/bitcoin/bitcoin/pull/28175#discussion_r1278081971)
I mentioned these specifically because:
1. ChatGPT is the most popularly known, and most likely to be searched for if someone is considering using it.
2. GitHub promotes use of Copilot heavily, and we are using GitHub.
3. Meta is falsely advertising LLaMA as open source, and many people are just believing that without verifying. (The source code is not available, and the license is not permissive)
💬 achow101 commented on pull request "script: check op_verif and op_vernotif":
(https://github.com/bitcoin/bitcoin/pull/28169#issuecomment-1656430144)
Concept ACK
I agree that `BAD_OPCODE` is the wrong error for these opcodes. However, instead of adding a new opcode, I think these should just return the existing `DISABLED_OPCODE` error that is used for the other disabled opcodes.
Also `OP_VER` should be included here as well as that's really what is disabled. `OP_VERIF` and `OP_VERNOTIF` are really just variants of `OP_VER`.
(https://github.com/bitcoin/bitcoin/pull/28169#issuecomment-1656430144)
Concept ACK
I agree that `BAD_OPCODE` is the wrong error for these opcodes. However, instead of adding a new opcode, I think these should just return the existing `DISABLED_OPCODE` error that is used for the other disabled opcodes.
Also `OP_VER` should be included here as well as that's really what is disabled. `OP_VERIF` and `OP_VERNOTIF` are really just variants of `OP_VER`.
💬 Dearfor commented on pull request "script: check op_verif and op_vernotif":
(https://github.com/bitcoin/bitcoin/pull/28169#issuecomment-1656471607)
Ok thanks.
(https://github.com/bitcoin/bitcoin/pull/28169#issuecomment-1656471607)
Ok thanks.
🤔 furszy reviewed a pull request: "wallet: bugfix, disallow migration of invalid scripts"
(https://github.com/bitcoin/bitcoin/pull/28125#pullrequestreview-1553154514)
Feedback tackled, thanks achow101.
Expanded test coverage with the requested label check and few more checks. Now the test:
1) Verifies that neither the invalid double sh script label, nor the `addr(<script_hash_addr>)` descriptor, are contained by any of the migrated wallets.
2) Verifies that the valid sh script (the original single sh script that is imported at the same time as the invalid one) and its address book record are contained by the migrated watch-only wallet.
Also, while was
...
(https://github.com/bitcoin/bitcoin/pull/28125#pullrequestreview-1553154514)
Feedback tackled, thanks achow101.
Expanded test coverage with the requested label check and few more checks. Now the test:
1) Verifies that neither the invalid double sh script label, nor the `addr(<script_hash_addr>)` descriptor, are contained by any of the migrated wallets.
2) Verifies that the valid sh script (the original single sh script that is imported at the same time as the invalid one) and its address book record are contained by the migrated watch-only wallet.
Also, while was
...
💬 furszy commented on pull request "wallet: bugfix, disallow migration of invalid scripts":
(https://github.com/bitcoin/bitcoin/pull/28125#discussion_r1278210618)
> Perhaps check that this label doesn't appear in any of the migrated wallets?
Sure, expanded the test to cover it, and also added few other additional cases.
(https://github.com/bitcoin/bitcoin/pull/28125#discussion_r1278210618)
> Perhaps check that this label doesn't appear in any of the migrated wallets?
Sure, expanded the test to cover it, and also added few other additional cases.
💬 ariard commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1278215639)
So if my understanding of the API is correct, you receive a package from `AcceptPackage()`, and there is a call to the constructor `AncestorPackage()`, then we verify each transaction being component of the package is in the mempool by wtxid or txid or `PreChecks()` them.
If the transaction is `PreChecks()` valid, then we call `AddFeeAndVsize()`. If we have a `TX_SINGLE_FAILURE`, we add `AddFeeAndVsize()` or if we have a `TX_MISSING_INPUTS`, we `SkipWithDescendants()` the transaction. All oth
...
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1278215639)
So if my understanding of the API is correct, you receive a package from `AcceptPackage()`, and there is a call to the constructor `AncestorPackage()`, then we verify each transaction being component of the package is in the mempool by wtxid or txid or `PreChecks()` them.
If the transaction is `PreChecks()` valid, then we call `AddFeeAndVsize()`. If we have a `TX_SINGLE_FAILURE`, we add `AddFeeAndVsize()` or if we have a `TX_MISSING_INPUTS`, we `SkipWithDescendants()` the transaction. All oth
...
💬 hebasto commented on pull request "ci: Run Windows native task on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28173#issuecomment-1656668537)
> In the meantime it may be good to do 10 runs and then check how many of them fail
Observing intermittent "Error: no RPC connection". Converting this PR to a draft for now
(https://github.com/bitcoin/bitcoin/pull/28173#issuecomment-1656668537)
> In the meantime it may be good to do 10 runs and then check how many of them fail
Observing intermittent "Error: no RPC connection". Converting this PR to a draft for now
📝 hebasto converted_to_draft a pull request: "ci: Run Windows native task on GitHub Actions"
(https://github.com/bitcoin/bitcoin/pull/28173)
From https://github.com/bitcoin/bitcoin/issues/28098:
> Thus, someone would have to sponsor an amount of roughly 5kUSD/mo for those two tasks.
> If the goal is to stay on a free plan, I think the only option is GitHub Actions CI.
Historical context:
- https://github.com/bitcoin/bitcoin/pull/17697
- https://github.com/bitcoin/bitcoin/issues/17803
- https://github.com/bitcoin/bitcoin/pull/18031
Security concerns:
- https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-16514321
...
(https://github.com/bitcoin/bitcoin/pull/28173)
From https://github.com/bitcoin/bitcoin/issues/28098:
> Thus, someone would have to sponsor an amount of roughly 5kUSD/mo for those two tasks.
> If the goal is to stay on a free plan, I think the only option is GitHub Actions CI.
Historical context:
- https://github.com/bitcoin/bitcoin/pull/17697
- https://github.com/bitcoin/bitcoin/issues/17803
- https://github.com/bitcoin/bitcoin/pull/18031
Security concerns:
- https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-16514321
...
💬 Sjors commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#discussion_r1278285111)
Because future developers won't (easily) know where the number comes from.
(https://github.com/bitcoin/bitcoin/pull/28130#discussion_r1278285111)
Because future developers won't (easily) know where the number comes from.
💬 Sjors commented on pull request "CONTRIBUTING: Caution against using AI/LLMs (ChatGPT, Copilot, etc)":
(https://github.com/bitcoin/bitcoin/pull/28175#discussion_r1278285397)
I think it's fine to mention these examples.
(https://github.com/bitcoin/bitcoin/pull/28175#discussion_r1278285397)
I think it's fine to mention these examples.
💬 hsjoberg commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1656705552)
Concept ACK.
This is helpful for second later protocols, including Lightning.
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1656705552)
Concept ACK.
This is helpful for second later protocols, including Lightning.
💬 Sjors commented on pull request "CONTRIBUTING: Caution against using AI/LLMs (ChatGPT, Copilot, etc)":
(https://github.com/bitcoin/bitcoin/pull/28175#issuecomment-1656707196)
Concept ACK, but happy to wait for legal opinions. Hopefully they clarify the risks in two separate categories:
1. Content the AI obtained somewhere else without permission (i.e. claims from the original author)
2. Content the AI generated itself. Potentially owned by some corporation who didn't give permission to the person making the pull request to MIT license it.
When it comes to (1) I'm more worried about snippets of fresh code than e.g. suggested refactorings. I don't see how one
...
(https://github.com/bitcoin/bitcoin/pull/28175#issuecomment-1656707196)
Concept ACK, but happy to wait for legal opinions. Hopefully they clarify the risks in two separate categories:
1. Content the AI obtained somewhere else without permission (i.e. claims from the original author)
2. Content the AI generated itself. Potentially owned by some corporation who didn't give permission to the person making the pull request to MIT license it.
When it comes to (1) I'm more worried about snippets of fresh code than e.g. suggested refactorings. I don't see how one
...
📝 hebasto opened a pull request: "qa, doc: Fix comment"
(https://github.com/bitcoin/bitcoin/pull/28181)
This PR is a follow-up for:
- https://github.com/bitcoin/bitcoin/pull/9956
- https://github.com/bitcoin/bitcoin/pull/10096
(https://github.com/bitcoin/bitcoin/pull/28181)
This PR is a follow-up for:
- https://github.com/bitcoin/bitcoin/pull/9956
- https://github.com/bitcoin/bitcoin/pull/10096
💬 petertodd commented on pull request "CONTRIBUTING: Caution against using AI/LLMs (ChatGPT, Copilot, etc)":
(https://github.com/bitcoin/bitcoin/pull/28175#issuecomment-1656713175)
ACK
The copyright lobby is pretty strong, and stands to lose a lot from AI. I think there's a significant chance that AI copyright gets resolved in favor of copyright owners in such a way that is disastrous for AI. Just look at how the copyright lobby managed to keep extending the duration of copyrights worldwide to ludicrious, economically irrational, lengths until very recently.
Also, AI poses unknown security threats. It frequently hallucinates incorrect answers. Bitcoin Core is a type
...
(https://github.com/bitcoin/bitcoin/pull/28175#issuecomment-1656713175)
ACK
The copyright lobby is pretty strong, and stands to lose a lot from AI. I think there's a significant chance that AI copyright gets resolved in favor of copyright owners in such a way that is disastrous for AI. Just look at how the copyright lobby managed to keep extending the duration of copyrights worldwide to ludicrious, economically irrational, lengths until very recently.
Also, AI poses unknown security threats. It frequently hallucinates incorrect answers. Bitcoin Core is a type
...
💬 petertodd commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#discussion_r1278289632)
> Dropping a constant but then hardcoding it all over the place
Where have I hardcoded it? I deliberately used a different constant in some of the updated tests to make clear that after this change, it's just a number. In the others - just 4 lines - I can of course just add a comment in those few lines explaining what the number is.
(https://github.com/bitcoin/bitcoin/pull/28130#discussion_r1278289632)
> Dropping a constant but then hardcoding it all over the place
Where have I hardcoded it? I deliberately used a different constant in some of the updated tests to make clear that after this change, it's just a number. In the others - just 4 lines - I can of course just add a comment in those few lines explaining what the number is.
💬 petertodd commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1656714337)
> Concept NACK unless you keep the default at `MAX_OP_RETURN_RELAY`. Depending on adoption we can always drop that default later. No need to make pull requests more controversial than necessary.
@ChristopherA is paying me $1000 for 10 hours work to do this pull-req. Since the other sub-change got mired in controversy, I decided to meet my client requirement in a time-efficient manner. Obviously, I can't promise to him that this will actually get merged. But I can promise to him that I'll do a
...
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1656714337)
> Concept NACK unless you keep the default at `MAX_OP_RETURN_RELAY`. Depending on adoption we can always drop that default later. No need to make pull requests more controversial than necessary.
@ChristopherA is paying me $1000 for 10 hours work to do this pull-req. Since the other sub-change got mired in controversy, I decided to meet my client requirement in a time-efficient manner. Obviously, I can't promise to him that this will actually get merged. But I can promise to him that I'll do a
...
💬 petertodd commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1656715854)
> From CI:
>
> ```
> test/functional/mempool_datacarrier.py:31: error: Bracketed expression "[...]" is not valid as a type [valid-type]
> test/functional/mempool_datacarrier.py:31: note: Did you mean "List[...]"?
> Found 1 error in 1 file (checked 269 source files)
> ^---- failure generated from lint-python.py
> ```
> From CI:
>
> ```
> test/functional/mempool_datacarrier.py:31: error: Bracketed expression "[...]" is not valid as a type [valid-type]
> test/functional/mempoo
...
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1656715854)
> From CI:
>
> ```
> test/functional/mempool_datacarrier.py:31: error: Bracketed expression "[...]" is not valid as a type [valid-type]
> test/functional/mempool_datacarrier.py:31: note: Did you mean "List[...]"?
> Found 1 error in 1 file (checked 269 source files)
> ^---- failure generated from lint-python.py
> ```
> From CI:
>
> ```
> test/functional/mempool_datacarrier.py:31: error: Bracketed expression "[...]" is not valid as a type [valid-type]
> test/functional/mempoo
...
💬 willcl-ark commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1656717430)
I’m on mobile and didn’t check the pull, but do you need to `from typing import List` too?
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1656717430)
I’m on mobile and didn’t check the pull, but do you need to `from typing import List` too?
⚠️ Billybeast01 opened an issue: "Flash bitcoin"
(https://github.com/bitcoin/bitcoin/issues/28182)
Flash bitcoin
(https://github.com/bitcoin/bitcoin/issues/28182)
Flash bitcoin
💬 Sjors commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#discussion_r1278293244)
In transaction_tests it's either the same number of +-1 which is confusing (can be solved with a comment), but I see in other places you made it 100 which is indeed more clear.
(https://github.com/bitcoin/bitcoin/pull/28130#discussion_r1278293244)
In transaction_tests it's either the same number of +-1 which is confusing (can be solved with a comment), but I see in other places you made it 100 which is indeed more clear.
💬 Sjors commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1656723149)
> So frankly, at this point I don't care to split it up into two pull-reqs for such a minor reason.
Making a significant change to default policy is not a minor reason.
> is paying me $1000 for 10 hours work to do this pull-req
That's great, but others have to spend hours reviewing this stuff, plus occasionally having to deal with the brigading. I'm more likely to do that for a change that keeps the default, but I'm not the only one who can review of course.
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1656723149)
> So frankly, at this point I don't care to split it up into two pull-reqs for such a minor reason.
Making a significant change to default policy is not a minor reason.
> is paying me $1000 for 10 hours work to do this pull-req
That's great, but others have to spend hours reviewing this stuff, plus occasionally having to deal with the brigading. I'm more likely to do that for a change that keeps the default, but I'm not the only one who can review of course.