Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1847219113)
Added a comment to this effect.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1847234154)
As discussed [on IRC](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2024-11-15), I've moved all sPK-specific info into a substructure that matches the one used in `getrawtransaction 2`. I've added @instagibbs' test under a commit with him as co-author.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1847218317)
Added tests including duplicate and reversal.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1847250753)
Maybe could come as a follow-up?
🤔 furszy reviewed a pull request: "test: Introduce ensure_for helper"
(https://github.com/bitcoin/bitcoin/pull/30893#pullrequestreview-2443695664)
utACK 111465d72dd35e42361fc2a089036f652417ed37
💬 TheCharlatan commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1847279135)
Good catch, thank you for taking another look! I'm also not sure what to do in light of this. At the minimum this reason for the duplicate check should be documented. I feel like with this kind of change I would err on the conservative side and not introduce potentially breaking behaviour. Even if this might be a nice way to inject block data into the node, I don't know if this is something anybody needs, even if it now more closely mimics `getblockfrompeer`.

We could keep the duplicate chec
...
💬 TheCharlatan commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1846964232)
Why is this not initialized again with `SCRIPT_ERR_UNKNOWN_ERROR`? I guess it is redundant, because it is set in the verification functions, but might still be good to keep it as a guard?
💬 TheCharlatan commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1847292854)
Isn't this the consensus only result?
⚠️ MORTEZAMIRSALI opened an issue: "Officials of the organization pay attention, just as you guarantee health here, how do you not prevent the invasion of wild animals into the reservoirs related to me? This double action means that no person, whether he was already working or later, enters, or no organization has the right to interfere in the related reservoirs. They don't have the original owner in your repositories, destroy my assets, Twitter changed all my tokens, took my possessions, on the other hand, pay attention to the jackals' support, there have been several crashes today, 400 bitcoins have been mined, destroy them all, why without the permission of the owner, the right to enter address and interests, you should organize and make laws according to my property, everyone is in line to destroy, even tonspn is not "
(https://github.com/bitcoin/bitcoin/issues/31319)
💬 MORTEZAMIRSALI commented on issue "Officials of the organization pay attention, just as you guarantee health here, how do you not prevent the invasion of wild animals into the reservoirs related to me? This double action means that no person, whether he was already working or later, enters, or no organization has the right to interfere in the related reservoirs. They don't have the original owner in your repositories, destroy my assets, Twitter changed all my tokens, took my possessions, on the other hand, pay attention to the jackals' support, there have been several crashes today, 400 bitcoins have been mined, destroy them all, why without the permission of the owner, the right to enter address and interests, you should organize and make laws according to my property, everyone is in line to destroy, even tonspn is not ":
(https://github.com/bitcoin/bitcoin/issues/31319#issuecomment-2484216000)
balanced and the main owner's financial interests are targeted, Bitcoin is the same, please check all the vandalism of the reservoirs, no one who uses blockchain without the permission of the owner How can the principal destroy interests?
achow101 closed an issue: "Officials of the organization pay attention, just as you guarantee health here, how do you not prevent the invasion of wild animals into the reservoirs related to me? This double action means that no person, whether he was already working or later, enters, or no organization has the right to interfere in the related reservoirs. They don't have the original owner in your repositories, destroy my assets, Twitter changed all my tokens, took my possessions, on the other hand, pay attention to the jackals' support, there have been several crashes today, 400 bitcoins have been mined, destroy them all, why without the permission of the owner, the right to enter address and interests, you should organize and make laws according to my property, everyone is in line to destroy, even tonspn is not "
(https://github.com/bitcoin/bitcoin/issues/31319)
:lock: achow101 locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/31319)
🤔 hodlinator reviewed a pull request: "util: Improve documentation and negation of args"
(https://github.com/bitcoin/bitcoin/pull/31212#pullrequestreview-2443807751)
Thanks for continued review and reminding me to raise the bar for the PR @ryanofsky.

Added tests which helped uncover that *bitcoin-cli* might as well also support `-norpccookiefile` too, and part of *src/common/init.cpp* which was not handling `-noconf` correctly.
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1847330326)
Fixed in latest push. Also switched from `ifstream::peek()` to `fs::is_directory()` after verifying the latter returns true for directory-symlinks as well.
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1847326060)
Thanks! Implemented in latest push with some variation.
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1847328295)
Thanks for catching this. Was not my intention to leave the door wide open when users pass `-norpccookiefile`. :man_facepalming:
Keep a belt & suspenders `bool` in latest push.
💬 kevkevinpal commented on pull request "test: Revert to random path element":
(https://github.com/bitcoin/bitcoin/pull/31317#issuecomment-2484236327)
tACK [fa80b08](https://github.com/bitcoin/bitcoin/pull/31317/commits/fa80b08fef0eaa600339caa678fdf80a8aec3ce3)

was able to reproduce the crash from https://github.com/bitcoin/bitcoin/pull/31317#issuecomment-2483056878 locally on my machine. Reviewed and ran the code aswell and it looks good to me
🤔 furszy reviewed a pull request: "Improve parallel script validation error debug logging"
(https://github.com/bitcoin/bitcoin/pull/31112#pullrequestreview-2443818466)
Concept ACK. Getting deeper.
💬 furszy commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1847332068)
nit: could also test that only the first found error is retrieved. Checking in this way that no further `check()`s are executed.
💬 hodlinator commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1847351945)
I am still working on this PR. Not thrilled about removing the 3 comment strings predating this PR.