< bitcoin-git>
bitcoin/master d103484 Hennadii Stepanov: util: Do not use gArgs global in ArgsManager member functions
< bitcoin-git>
bitcoin/master 283a73d MarcoFalke: Merge #20092: util: Do not use gArgs global in ArgsManager member function...
< bitcoin-git>
[bitcoin] MarcoFalke merged pull request #20092: util: Do not use gArgs global in ArgsManager member functions (master...201006-gargs) https://github.com/bitcoin/bitcoin/pull/20092
< wumpus>
aj: would it make sense to maintain that project board as part of the bitcoin/bitcoin repo? or do you prefer hosting it on your own repo
< wumpus>
(fwiw: I just closed the ancient "P2P refactor" project)
< wumpus>
great job in any case it's good to have an overview
< vasild>
hebasto: I dont think there would be any benefit of sending the sendaddrv2 message before verack (right now in #19954 we send it after verack)
< vasild>
there would be benefit if we send it before the version message (:-O) because then in the version message we could encode the addrMe and addrYou in the new format thus they can be e.g. torv3 addresses
< hebasto>
am I wrong considering the window between version and verack as the main venue for adding new feature negotiation now / in the future?
< vasild>
Also, if we push now a new message before verack, even though #19723 is merged, about 70% of the peers will misbehave us because they dont run the latest version
< vasild>
but for sendaddrv2 in particular it does not make a difference whether it is before or after verack
< hebasto>
debian has gcc 10.2 now; is it patched already?
< luke-jr>
"Correct handling of constant representations containing embedded nuls."
< luke-jr>
check changelog for that
< luke-jr>
hmm, changelog looks useless
< luke-jr>
hebasto: no, it looks like it's not fixed in 10.2.0
< luke-jr>
if inline_expand_builtin_.*cmp finds inline_expand_builtin_string_cmp, it's broken; if inline_expand_builtin_bytecmp, it's fixed
< luke-jr>
(unless Debian patched it some other way)
< hebasto>
luke-jr: thanks
< achow101>
ryanofsky: would you like to have a dicussion about wallet.dat vs wallet.sqlite?
< luke-jr>
IMO the latter is better, since users shouldn't be messing with wallet internal files, and backupwallet lets the user specify any extension anyway
< achow101>
luke-jr: but there's a lot of tooling and documentation that refer to the wallet.dat file
< luke-jr>
achow101: and users can still call their backups wallet.dat
< achow101>
also, I'm concerned that a downgrade would result in a new wallet.dat being created which would not be good
< achow101>
I should probably test that..
< luke-jr>
hmm
< achow101>
luke-jr: right, but if they have a script backing up "wallet.dat", then it wouldn't work anymore
< luke-jr>
achow101: yes it would
< luke-jr>
you could *never* copy the original wallet.dat directly
< luke-jr>
only via backupwallet RPC
< luke-jr>
which lets them specify any filename
< achow101>
???
< achow101>
you just go into the datadir and copy the file
< luke-jr>
no, that's not a supported method to backup and can result in corrupt backups
< achow101>
sure, but that doesn't mean people don't do it
< luke-jr>
sounds like a reason to break it
< luke-jr>
better to get an error on upgrade, than find out later your backups were corrupt
< luke-jr>
otoh, sqlite might not have the same corruption issue.. so..
< achow101>
likewise there may be a restore script that operates on wallet.dat files
< achow101>
also, backupwallet allowing you to specify an extension may interact poorly with this
< luke-jr>
true, we don't have a sane recover-from-backup
< achow101>
actually it probably doesn't since we do look for the file magic to deterine type, not the filename
< sipa>
luke-jr: backing uo wallet.dat by copying after a clean shutdown is soemthing i'd consider supported
< achow101>
it's also a lot more common for people to suggest copying the wallet.dat file for backups rather than using the backupwallet rpc
< achow101>
it's probably easier for people to understand file copying than it is for them to understand rpcs
< phantomcircuit>
luke-jr, unfortunately lots of people just copy the wallet.dat directly cause terrible tutorials
< luke-jr>
again, those are reasons to break such bad scripts/advice ;)
< luke-jr>
backward compatibility, and guides to restoring wallets, are the reasons I see to leave .dat
< achow101>
it seems like at least 0.19 and 0.20 expect a wallet.dat file in an existing dir for a wallet, so renaming to wallet.sqlite wouldn't silently fail on a downgrade
< sipa>
achow101: what happens if you have an sqlite wallet.dat and run an old version? will it try to do salvage/recover somehow?
< sipa>
or suggest to
< achow101>
sipa: no, it errors saying it isn' a bdb file
< achow101>
it checks the file magic before attempting to do anything to it
< sipa>
ok
< achow101>
hmm, actually it seems like we do actually attempt to salvage them, but bdb should be doing nothing because its the wrong magic
< achow101>
mmm, but wait, there's more, if the wallet is specified as a config option, 0.20 will make the wallet.dat file if it doesn't exist
< sanket1729>
How do I get my tests written in script_tests.cpp into script_test.json.gen?
< sipa>
// Uncomment if you want to output updated JSON tests.
< sipa>
// #define UPDATE_JSON_TESTS
< sanket1729>
Thanks.
< gwillen>
I am concerned that #20051 is a sign of things to come, I think the "no loaded wallet" error message needs to clearly spell out the fact that default wallet creation was deprecated and you should call createwallet
< gwillen>
(by 'things to come' I just mean 'once this gets released, a ton of people are going to be confused by the breakage)
< achow101>
The error message could be written better
< gwillen>
I don't know how long RPC error messages are allowed to get, in terms of explanations or pointers to documentation
< sipa>
at least this long: "Using bumpfee with wallets that have private keys disabled is deprecated. Use psbtbumpfee instead or restart bitcoind with -deprecatedrpc=bumpfee. This functionality will be removed in 0.22"
< gwillen>
this is possibly too long: "Wallet RPCs are disabled because no wallet is loaded. (NOTE: Bitcoin Core no longer creates a default wallet automatically. Load a wallet using `loadwallet`, or by running bitcoind with `-wallet`; or create a wallet with `createwallet`.)"
< gwillen>
but that would spell out the general options. (Alternatively, replace the last bit with a link to documentation, if there's a stable link that would work.)
< gwillen>
alternatively, is there any precedent for having "help foo" entries where foo is not an RPC? If so, we could refer people to "help default_wallet_deprecation" or so.
< achow101>
I think it's fine to just say what people can do
< achow101>
Maybe link to release notes if necessary