💬 Sjors commented on pull request "Make (Read/Write)BinaryFile work with char vector, use AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29229#discussion_r1466160913)
@vasild I would also prefer to fix things in AutoFile, but perhaps add a few comments to explain what happens?
(https://github.com/bitcoin/bitcoin/pull/29229#discussion_r1466160913)
@vasild I would also prefer to fix things in AutoFile, but perhaps add a few comments to explain what happens?
💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1466162584)
It was intentionally vague, because I might just be doing something else wrong. I'll read @maflcko's suggestion.
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1466162584)
It was intentionally vague, because I might just be doing something else wrong. I'll read @maflcko's suggestion.
💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1466164116)
Without that command I got the same behavior as if the command wasn't there. Presumably it's asynchronous and we somehow have to wait for it. Maybe the above solution avoids the problem altogether.
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1466164116)
Without that command I got the same behavior as if the command wasn't there. Presumably it's asynchronous and we somehow have to wait for it. Maybe the above solution avoids the problem altogether.
💬 Sjors commented on pull request "CKey: add Serialize and Unserialize":
(https://github.com/bitcoin/bitcoin/pull/29295#discussion_r1466165495)
Good point.
(https://github.com/bitcoin/bitcoin/pull/29295#discussion_r1466165495)
Good point.
💬 maflcko commented on issue "Unable to open wallet UI with ubuntu 23.10":
(https://github.com/bitcoin/bitcoin/issues/29311#issuecomment-1909861017)
> Also I follow this https://github.com/bitcoin/bitcoin/blob/master/doc/build-unix.md#dependency-build-instructions
There is no need to install dependencies for already packaged Bitcoin Core, since it has the dependencies bundled.
> Fontconfig warning: FcPattern object weight does not accept value [0 205) [1]
This issue tracker does not provide fontconfig support, but you can try a search engine of your choice to track down the issue and solutions. It may be related to the fontconfi
...
(https://github.com/bitcoin/bitcoin/issues/29311#issuecomment-1909861017)
> Also I follow this https://github.com/bitcoin/bitcoin/blob/master/doc/build-unix.md#dependency-build-instructions
There is no need to install dependencies for already packaged Bitcoin Core, since it has the dependencies bundled.
> Fontconfig warning: FcPattern object weight does not accept value [0 205) [1]
This issue tracker does not provide fontconfig support, but you can try a search engine of your choice to track down the issue and solutions. It may be related to the fontconfi
...
💬 fanquake commented on issue "Unable to open wallet UI with ubuntu 23.10":
(https://github.com/bitcoin/bitcoin/issues/29311#issuecomment-1909863507)
Close or move to GUI repo? cc @hebasto
(https://github.com/bitcoin/bitcoin/issues/29311#issuecomment-1909863507)
Close or move to GUI repo? cc @hebasto
💬 maflcko commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1466174158)
Looks like https://packages.ubuntu.com/noble/podman and https://packages.debian.org/trixie/podman includes the fixed `--replace`, so might try that at one point.
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1466174158)
Looks like https://packages.ubuntu.com/noble/podman and https://packages.debian.org/trixie/podman includes the fixed `--replace`, so might try that at one point.
💬 mmaryo commented on issue "Unable to open wallet UI with ubuntu 23.10":
(https://github.com/bitcoin/bitcoin/issues/29311#issuecomment-1909908601)
Hello
Thank you
this command line works
https://github.com/keshavbhatt/olivia/issues/95#issuecomment-774747492
But my wallet is empty?
May I find it somewhere else?
Are there logs?
(https://github.com/bitcoin/bitcoin/issues/29311#issuecomment-1909908601)
Hello
Thank you
this command line works
https://github.com/keshavbhatt/olivia/issues/95#issuecomment-774747492
But my wallet is empty?
May I find it somewhere else?
Are there logs?
💬 maflcko commented on issue "Unable to open wallet UI with ubuntu 23.10":
(https://github.com/bitcoin/bitcoin/issues/29311#issuecomment-1909918786)
> May I find it somewhere else?
If a backup exists, you can restore it from there.
> Are there logs?
There is a `debug.log` in the datadir (generally `.bitcoin`) which shows when Bitcoin Core was started, and possibly wallet logs.
(https://github.com/bitcoin/bitcoin/issues/29311#issuecomment-1909918786)
> May I find it somewhere else?
If a backup exists, you can restore it from there.
> Are there logs?
There is a `debug.log` in the datadir (generally `.bitcoin`) which shows when Bitcoin Core was started, and possibly wallet logs.
💬 vasild commented on pull request "Make (Read/Write)BinaryFile work with char vector, use AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29229#discussion_r1466212055)
This could happen:
1. fwrite() succeeds, returns ok the caller, some of the data is buffered in the OS and not yet on disk
2. fclose() is called, it tries to fflush() the buffered data to disk but fails due to IO error. The caller ignores the error returned by fclose()
3. the program continues with the wrong assumption that the data is safely on disk
(https://github.com/bitcoin/bitcoin/pull/29229#discussion_r1466212055)
This could happen:
1. fwrite() succeeds, returns ok the caller, some of the data is buffered in the OS and not yet on disk
2. fclose() is called, it tries to fflush() the buffered data to disk but fails due to IO error. The caller ignores the error returned by fclose()
3. the program continues with the wrong assumption that the data is safely on disk
💬 stickies-v commented on pull request "doc: clarify `BroadcastTransaction` comment":
(https://github.com/bitcoin/bitcoin/pull/29308#discussion_r1466213307)
I don't think we care about which specific RPCs call it? Would just mention that it can be called by RPC, that's the key info - and more maintainable.
```suggestion
// BroadcastTransaction can be called by RPC or by the wallet.
```
(https://github.com/bitcoin/bitcoin/pull/29308#discussion_r1466213307)
I don't think we care about which specific RPCs call it? Would just mention that it can be called by RPC, that's the key info - and more maintainable.
```suggestion
// BroadcastTransaction can be called by RPC or by the wallet.
```
💬 kristapsk commented on pull request "Add a `-permitbarepubkey` option":
(https://github.com/bitcoin/bitcoin/pull/29309#issuecomment-1909949991)
Concept ACK.
> there's no point of adding this if default policy is with P2PK enabled
There is, it gives node runners choice.
(https://github.com/bitcoin/bitcoin/pull/29309#issuecomment-1909949991)
Concept ACK.
> there's no point of adding this if default policy is with P2PK enabled
There is, it gives node runners choice.
💬 vasild commented on pull request "doc: clarify `BroadcastTransaction` comment":
(https://github.com/bitcoin/bitcoin/pull/29308#discussion_r1466224985)
_(this is heading in the direction of "amount of discussion is [inversely proportional](https://bikeshed.com/) to the complexity of the change")_
When working on another PR, I found this comment useful and I cared with RPC called the function. Surely there are other means to check who is calling a given function, but this comment gives a quick concise answer at a glance. To be useful it has to be precise and elaborate. If it is vague then it is less useful and it might as well be removed comp
...
(https://github.com/bitcoin/bitcoin/pull/29308#discussion_r1466224985)
_(this is heading in the direction of "amount of discussion is [inversely proportional](https://bikeshed.com/) to the complexity of the change")_
When working on another PR, I found this comment useful and I cared with RPC called the function. Surely there are other means to check who is calling a given function, but this comment gives a quick concise answer at a glance. To be useful it has to be precise and elaborate. If it is vague then it is less useful and it might as well be removed comp
...
💬 maflcko commented on issue "Huge disk fragmentation":
(https://github.com/bitcoin/bitcoin/issues/29296#issuecomment-1909970643)
According to your debug log 5 blocks are advanced per second, which seems fast, even for spinning storage. Not sure if much can be done to improve upon that. In any case, if you have a software solution, no one is holding you back from testing and benchmarking it. If it does really does improve performance significantly, it can be considered.
(https://github.com/bitcoin/bitcoin/issues/29296#issuecomment-1909970643)
According to your debug log 5 blocks are advanced per second, which seems fast, even for spinning storage. Not sure if much can be done to improve upon that. In any case, if you have a software solution, no one is holding you back from testing and benchmarking it. If it does really does improve performance significantly, it can be considered.
💬 stickies-v commented on pull request "doc: clarify `BroadcastTransaction` comment":
(https://github.com/bitcoin/bitcoin/pull/29308#discussion_r1466233109)
Extending that logic, should we document all the call sites of each helper function that's used by RPCs because it can be helpful sometimes? That seems abnormal to me. I understood the only relevance of this line to be in relation to the next one: "chainman, mempool and peerman are initialized before the RPC server and wallet are started". For this, it doesn't matter which RPCs call it.
(https://github.com/bitcoin/bitcoin/pull/29308#discussion_r1466233109)
Extending that logic, should we document all the call sites of each helper function that's used by RPCs because it can be helpful sometimes? That seems abnormal to me. I understood the only relevance of this line to be in relation to the next one: "chainman, mempool and peerman are initialized before the RPC server and wallet are started". For this, it doesn't matter which RPCs call it.
💬 kristapsk commented on pull request "doc: clarify `BroadcastTransaction` comment":
(https://github.com/bitcoin/bitcoin/pull/29308#discussion_r1466236373)
`BroadcastTransaction()` is not just any helper function, it may irreversibly affect user's privacy.
(https://github.com/bitcoin/bitcoin/pull/29308#discussion_r1466236373)
`BroadcastTransaction()` is not just any helper function, it may irreversibly affect user's privacy.
📝 hebasto opened a pull request: "ci: Update cache action"
(https://github.com/bitcoin/bitcoin/pull/29313)
This PR fixes deprecation [warnings](https://github.com/bitcoin/bitcoin/actions/runs/7652979339) for Node.js 16 actions in the GHA CI:
See:
- https://github.com/marketplace/actions/cache
- https://github.com/actions/cache/releases/tag/v4.0.0
(https://github.com/bitcoin/bitcoin/pull/29313)
This PR fixes deprecation [warnings](https://github.com/bitcoin/bitcoin/actions/runs/7652979339) for Node.js 16 actions in the GHA CI:
See:
- https://github.com/marketplace/actions/cache
- https://github.com/actions/cache/releases/tag/v4.0.0
💬 Sjors commented on pull request "Make (Read/Write)BinaryFile work with char vector, use AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29229#discussion_r1466268617)
That's not good. I guess we also don't want to sync to disk for every field that's `>>`'d to a file though.
(https://github.com/bitcoin/bitcoin/pull/29229#discussion_r1466268617)
That's not good. I guess we also don't want to sync to disk for every field that's `>>`'d to a file though.
💬 maflcko commented on pull request "ci: Use DEBUG=1 in depends for MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27495#discussion_r1466284981)
might as well use 18rc1, which will be tagged tomorrow?
(https://github.com/bitcoin/bitcoin/pull/27495#discussion_r1466284981)
might as well use 18rc1, which will be tagged tomorrow?
✅ maflcko closed an issue: "test: use-of-uninitialized-value in sqlite3Strlen30"
(https://github.com/bitcoin/bitcoin/issues/27222)
(https://github.com/bitcoin/bitcoin/issues/27222)