💬 darosior commented on pull request "Wallet: estimate the size of signed inputs using descriptors":
(https://github.com/bitcoin/bitcoin/pull/26567#discussion_r1180304523)
Pushed a much cleaner version of this: introducing a `MultiSigningProvider` to wrap around both the wallet and external providers. Still as a commit on top, let me know what you think @achow101.
(https://github.com/bitcoin/bitcoin/pull/26567#discussion_r1180304523)
Pushed a much cleaner version of this: introducing a `MultiSigningProvider` to wrap around both the wallet and external providers. Still as a commit on top, let me know what you think @achow101.
📝 fjahr opened a pull request: "test: Remove modinv python util helper function"
(https://github.com/bitcoin/bitcoin/pull/27538)
Since #27483 was merged the `modinv()` body is just one line calling pythons own implementation of `pow()`. We can just remove the function as it doesn't seem to add any value. Additionally the comment in the function is now outdated and the test is only testing two ways of doing modular inverse but both using python's `pow()` function.
(https://github.com/bitcoin/bitcoin/pull/27538)
Since #27483 was merged the `modinv()` body is just one line calling pythons own implementation of `pow()`. We can just remove the function as it doesn't seem to add any value. Additionally the comment in the function is now outdated and the test is only testing two ways of doing modular inverse but both using python's `pow()` function.
💬 sipa commented on pull request "test: Remove modinv python util helper function":
(https://github.com/bitcoin/bitcoin/pull/27538#issuecomment-1527470660)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/27538#issuecomment-1527470660)
Concept ACK
💬 MarcoFalke commented on pull request "test: Remove modinv python util helper function":
(https://github.com/bitcoin/bitcoin/pull/27538#discussion_r1180329594)
```
test/functional/test_framework/util.py:18:1: F401 'unittest' imported but unused
(https://github.com/bitcoin/bitcoin/pull/27538#discussion_r1180329594)
```
test/functional/test_framework/util.py:18:1: F401 'unittest' imported but unused
💬 MarcoFalke commented on pull request "test: Remove modinv python util helper function":
(https://github.com/bitcoin/bitcoin/pull/27538#discussion_r1180329919)
```
test/functional/test_framework/util.py:18:1: F401 'unittest' imported but unused
(https://github.com/bitcoin/bitcoin/pull/27538#discussion_r1180329919)
```
test/functional/test_framework/util.py:18:1: F401 'unittest' imported but unused
💬 MarcoFalke commented on pull request "lint: stop ignoring LIEF imports":
(https://github.com/bitcoin/bitcoin/pull/27507#issuecomment-1527477037)
```
contrib/devtools/security-check.py:246: error: "object" has no attribute "header" [attr-defined]
(https://github.com/bitcoin/bitcoin/pull/27507#issuecomment-1527477037)
```
contrib/devtools/security-check.py:246: error: "object" has no attribute "header" [attr-defined]
💬 Riahiamirreza commented on issue "rpc: show P2(W)SH redeemScript in getrawtransaction (and friends)":
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1527485466)
Can I work on it? Isn't it much hard as the first time contribution?
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1527485466)
Can I work on it? Isn't it much hard as the first time contribution?
💬 michaelfolkson commented on issue "rpc: show P2(W)SH redeemScript in getrawtransaction (and friends)":
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1527511935)
@Riahiamirreza: You don't need to ask but stating you are planning to work on this is helpful for others. If you look at the linked attempt in #8849 it looks doable as a first contribution but I don't know your skillset, level of understanding.
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1527511935)
@Riahiamirreza: You don't need to ask but stating you are planning to work on this is helpful for others. If you look at the linked attempt in #8849 it looks doable as a first contribution but I don't know your skillset, level of understanding.
💬 kevkevinpal commented on pull request "test: Remove modinv python util helper function":
(https://github.com/bitcoin/bitcoin/pull/27538#issuecomment-1527515641)
Concept ACK also did a grep for modinv in /test and looks like you got all them
(https://github.com/bitcoin/bitcoin/pull/27538#issuecomment-1527515641)
Concept ACK also did a grep for modinv in /test and looks like you got all them
💬 fjahr commented on pull request "test: Remove modinv python util helper function":
(https://github.com/bitcoin/bitcoin/pull/27538#discussion_r1180372914)
Thanks, fixed.
(https://github.com/bitcoin/bitcoin/pull/27538#discussion_r1180372914)
Thanks, fixed.
💬 Riahiamirreza commented on issue "rpc: show P2(W)SH redeemScript in getrawtransaction (and friends)":
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1527530419)
@michaelfolkson Thanks for your reply. I think I have a good background in c++ and I'm familiar with the source code. But to start is I really need help.
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1527530419)
@michaelfolkson Thanks for your reply. I think I have a good background in c++ and I'm familiar with the source code. But to start is I really need help.
💬 schildbach commented on pull request "Remove BIP35 mempool p2p message":
(https://github.com/bitcoin/bitcoin/pull/27426#issuecomment-1527540750)
The mempool message is used by all wallets based on bitcoinj to learn about pending transactions, in both full blocks mode and also in filtered mode. The estimated install base is 10M+.
(https://github.com/bitcoin/bitcoin/pull/27426#issuecomment-1527540750)
The mempool message is used by all wallets based on bitcoinj to learn about pending transactions, in both full blocks mode and also in filtered mode. The estimated install base is 10M+.
💬 michaelfolkson commented on issue "rpc: show P2(W)SH redeemScript in getrawtransaction (and friends)":
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1527543952)
@Riahiamirreza: Look at that attempt at changing rawtransaction.cpp and then ask questions on IRC #bitcoin-core-pr-reviews or Bitcoin StackExchange about the code in that file or the PR.
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1527543952)
@Riahiamirreza: Look at that attempt at changing rawtransaction.cpp and then ask questions on IRC #bitcoin-core-pr-reviews or Bitcoin StackExchange about the code in that file or the PR.
💬 Riahiamirreza commented on pull request "Updated the installation instructions for macOS":
(https://github.com/bitcoin/bitcoin/pull/27525#discussion_r1180401267)
nit, using colon at the end of the line may looks better
> From here, change your working directory to "bitcoin" by running:
(https://github.com/bitcoin/bitcoin/pull/27525#discussion_r1180401267)
nit, using colon at the end of the line may looks better
> From here, change your working directory to "bitcoin" by running:
👍 theStack approved a pull request: "test: Remove modinv python util helper function"
(https://github.com/bitcoin/bitcoin/pull/27538#pullrequestreview-1405943871)
ACK dc14ba08e6e502f3e31d935bcd053a287c6610ca
(https://github.com/bitcoin/bitcoin/pull/27538#pullrequestreview-1405943871)
ACK dc14ba08e6e502f3e31d935bcd053a287c6610ca
💬 MarcoFalke commented on issue "Proposed Timeline for Legacy Wallet and BDB removal":
(https://github.com/bitcoin/bitcoin/issues/20160#issuecomment-1527763221)
I think the milestones will need to be edited in OP to be moved back by one release?
(https://github.com/bitcoin/bitcoin/issues/20160#issuecomment-1527763221)
I think the milestones will need to be edited in OP to be moved back by one release?
💬 MarcoFalke commented on pull request "test: add coverage for `-bantime`":
(https://github.com/bitcoin/bitcoin/pull/26604#discussion_r1180578823)
Pretty sure this will lead to a race when the rpc is dispatched after a one second sleep?
(https://github.com/bitcoin/bitcoin/pull/26604#discussion_r1180578823)
Pretty sure this will lead to a race when the rpc is dispatched after a one second sleep?
💬 MarcoFalke commented on issue "ci: failure in Docker build step":
(https://github.com/bitcoin/bitcoin/issues/27492#issuecomment-1527772394)
For now the only workarounds I see are:
* rebase
* remove the cirrus CI env file feature for this task
(https://github.com/bitcoin/bitcoin/issues/27492#issuecomment-1527772394)
For now the only workarounds I see are:
* rebase
* remove the cirrus CI env file feature for this task
💬 jonatack commented on pull request "Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#issuecomment-1527773928)
> https://github.com/bitcoin/bitcoin/commit/81f29f03a607cbfb7162705c1c1618ca7b59640e I'm not sure if or when legacy wallet support is expected to be phased out, but if isactive is only different from ismine in legacy wallets, is it worth adding a field (and all the new methods), versus just updating the ismine field documentation in the help for descriptor wallets
Following up on https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1160272170 above, according to https://github.com/bitcoi
...
(https://github.com/bitcoin/bitcoin/pull/27216#issuecomment-1527773928)
> https://github.com/bitcoin/bitcoin/commit/81f29f03a607cbfb7162705c1c1618ca7b59640e I'm not sure if or when legacy wallet support is expected to be phased out, but if isactive is only different from ismine in legacy wallets, is it worth adding a field (and all the new methods), versus just updating the ismine field documentation in the help for descriptor wallets
Following up on https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1160272170 above, according to https://github.com/bitcoi
...
💬 brunoerg commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1527780479)
> One shot connections + immediate tx broadcast without inv,getdata exchange leaks that the tx is likely being broadcast for the first time (easy to censor the tx as the receiver).
I agree. My main concern is about how to ensure the transaction has not been censored. It seems easy for an attacker to identify someone opened a connection just to relay its own transaction.
> Now we wait until we receive an INV about that tx from somebody
Just to clarify: We would open the short-lived conne
...
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1527780479)
> One shot connections + immediate tx broadcast without inv,getdata exchange leaks that the tx is likely being broadcast for the first time (easy to censor the tx as the receiver).
I agree. My main concern is about how to ensure the transaction has not been censored. It seems easy for an attacker to identify someone opened a connection just to relay its own transaction.
> Now we wait until we receive an INV about that tx from somebody
Just to clarify: We would open the short-lived conne
...