<michaelfolkson>
sipa: Right, just a template sketched out roadmap over multiple major versions to have discussions around. Some people raised on the mailing list (reasonably imo) that we shouldn't attempt this given it isn't binding in any way, no authority to ensure it happens, hard to predict what should be in future major versions etc
Kaizen_Kintsugi has joined #bitcoin-core-dev
<michaelfolkson>
But some things should be phased in/out over multiple major versions (if they are to happen at all) so to attempt this it needs a sketched out roadmap which hopefully reviewers will engage with and provide feedback. Deprecation of BDB is in similar boat
AaronvanW has joined #bitcoin-core-dev
Kaizen_Kintsugi has quit [Ping timeout: 252 seconds]
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
<dougefish>
Is there any demand to speed up the CI runs? For example, 04_install.sh updates/installs ubuntu packages every run. One way to decrease runtime is to create a fixed docker image. The default image ubuntu:bionic in .cirrus.yml would be replaced by our own image (e.g. itcoin-core:ubuntu-$SHA1). This also provides reproducale build/CI results, as well as easier to run locally.
<dougefish>
One downside i see is maintaince of this new image and where to "push" it
dougefish has quit [Quit: dougefish]
dougefish has joined #bitcoin-core-dev
bomb-on has joined #bitcoin-core-dev
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] MarcoFalke closed pull request #21954: Some minor code changes to improve efficiency of processing TXs (master...TXcatches) https://github.com/bitcoin/bitcoin/pull/21954
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] MarcoFalke closed pull request #21841: Send fewer feefilter messages (avoid the wobbling number issue) (master...SteadierFeefilter) https://github.com/bitcoin/bitcoin/pull/21841
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] MarcoFalke closed pull request #21827: Display progress of LoadBlockDB() on splash screen (master...SplashLoadBlockProgress) https://github.com/bitcoin/bitcoin/pull/21827
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] MarcoFalke closed pull request #21618: WIP: Reduce MinRelayFee slower when Mempool utilised and faster when needed. (master...MinRelayFeeReductionChanges) https://github.com/bitcoin/bitcoin/pull/21618
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] dougEfresh opened pull request #23337: tests: Add assert_less_than and assert_less_than_or_equal to test framework (master...23119-assert_less) https://github.com/bitcoin/bitcoin/pull/23337
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
<bitcoin-git>
bitcoin/master 632aad9 Pieter Wuille: Make CAddrman::Select_ select buckets, not positions, first
<bitcoin-git>
bitcoin/master 58275db W. J. van der Laan: Merge bitcoin/bitcoin#23140: Make CAddrman::Select_ select buckets, not po...
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] laanwj merged pull request #23140: Make CAddrman::Select_ select buckets, not positions, first (master...202109_addrmanbias) https://github.com/bitcoin/bitcoin/pull/23140
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
<jnewbery>
thanks fanquake and laanwj!
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] MarcoFalke closed pull request #18902: Bugfix: Only use git for build info if the repository is actually the right one (master...fix_gitdir_again) https://github.com/bitcoin/bitcoin/pull/18902
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
mikehu44_ has quit [Ping timeout: 260 seconds]
mikehu44 has quit [Ping timeout: 260 seconds]
masta`` has joined #bitcoin-core-dev
Kaizen_Kintsugi has joined #bitcoin-core-dev
<real_or_random>
dougefish: one neat possibility for cirrus CI is that you can use Cirrus to build the image, let them store it and then use it to run CI
<real_or_random>
we do this in secp256k1. it works pretty well there. though I think it's not compatible with some other features
Kaizen_Kintsugi has quit [Ping timeout: 264 seconds]
<MarcoFalke>
dougefish: Our CI is platfrom agnostic. You should be able to run it on any CI provider, or even locally. I am not sure how much hassle this would be with self-made docker images.
ravan has quit [Quit: Leaving]
<MarcoFalke>
jonasschnelli: I was about to ask you whether you planned to remove your fingerprint from the "trusted-keys" for merging, but it looks like this will break verify-commits ...
<laanwj>
you would also have a add all his merge commits to exceptions, i guess
<laanwj>
or patch the script to allow different key for different ranges of commits
<bitcoin-git>
bitcoin/master a5595b1 Andrew Chow: tests: Remove global vCoins and testWallet from coinselector_tests
<bitcoin-git>
bitcoin/master 5e54aa9 Andrew Chow: bench: remove global testWallet from CoinSelection benchmark
<bitcoin-git>
bitcoin/master 9bf0243 Andrew Chow: bench: Use DescriptorScriptPubKeyMan for wallet things
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] laanwj merged pull request #23288: tests: remove usage of LegacyScriptPubKeyMan from some wallet tests (master...rm-testWallet-tests) https://github.com/bitcoin/bitcoin/pull/23288
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
<jonasschnelli>
MarcoFalke: I had no plan to remove my keyid,… would that make sense and how would you fix verify commits?
<jonasschnelli>
Ideally, we should set en expiration date next to those keyid
Guest54 has joined #bitcoin-core-dev
Guest54 has quit [Client Quit]
<laanwj>
intermittent error in unit tests: ./src/test/blockfilter_index_tests.cpp(140): fatal error: in "blockfilter_index_tests/blockfilter_index_initial_sync": critical check time_start + timeout_ms > GetTimeMillis() has failed
goatpig has quit [Remote host closed the connection]
<michaelfolkson>
This getting through the fog of what is a bug and what isn't, how it should be disclosed if it is, how we should deal with policy long term going forward etc is hard to wade through (and it isn't your fault, I'm unsure too)
gnaf has joined #bitcoin-core-dev
<michaelfolkson>
Btw I'm not suggesting you split into 2 PRs, just want to understand why you thought they should go together
<michaelfolkson>
[21:46:28] <achow101> #proposedwalletmeetingtopic feature_segwit.py's dependency on legacy wallet
Kaizen_Kintsugi has joined #bitcoin-core-dev
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] JeremyRubin closed pull request #22948: [Tests] Compute the Power Set of all flags instead of one by one exclusion (master...flag-powerset) https://github.com/bitcoin/bitcoin/pull/22948
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
<jeremyrubin>
does anyone have any feelings on #22954 and #22876?
<gribble>
https://github.com/bitcoin/bitcoin/issues/22954 | [TESTS] Allow tx_invalid.json tests to include flag rules for if_unset: [A,B,C] then_unset: [D] by JeremyRubin · Pull Request #22954 · bitcoin/bitcoin · GitHub
<gribble>
https://github.com/bitcoin/bitcoin/issues/22876 | [TESTS] Update Transaction Tests to permit setting a flag as always on and disabling the exhaustive failure test by JeremyRubin · Pull Request #22876 · bitcoin/bitcoin · GitHub
<jeremyrubin>
i recall sipa ack'd this approach in IRC a while back as being the lesser evil (of that, or making CTV valid in mempool before active)
<jeremyrubin>
but i don't think that approach ack extended to these testing changes
<achow101>
there is one preproposed topic, any other topics?
<achow101>
#topic feature_segwit.py's dependency on legacy wallet (achow101)
<core-meetingbot>
topic: feature_segwit.py's dependency on legacy wallet (achow101)
sipsorcery has joined #bitcoin-core-dev
<achow101>
feature_segwit.py is currently legacy wallet only test
<brunoerg>
hi
<achow101>
I was wondering if we thought it was important enough to refactor to not depend on the lgacy wallet
<gene>
hi
<achow101>
or if it was ok to just ditch it
<achow101>
it seems like a big chunk of it is testing legacy wallet only things
<achow101>
but there is also a part that tests segwit activation and non-wallet things
<sipa>
let me see
<michaelfolkson>
Is this resolved by extending the functionality of MiniWallet (longer term potentially)?
<achow101>
michaelfolkson: no, it's specifically testing weird IsMine behavior
<achow101>
and how the legacy wallet deals with imports, adding segwit addresses, etc.
<sipa>
as long as we have the legacy wallet, i think it's important that those things get tested
<S3RK>
is it covered by other tests?
<sipa>
but it should be separate from testing segwit consensus/activation logic
<achow101>
sipa: yes, but after legacy wallet removal?
<sipa>
what do you mean by removal?
<achow101>
sipa: I've been experimenting with removing the legacy wallet outright (except for migration) and seeing what other things will need to be done in order to get that through
<sipa>
that's useful i guess to see what unnecessarily depends on it
<sipa>
though, i hope we're not planning to actually drop support for legacy wallets anytime soon?
<sipa>
there is a difference between a test _of non-wallet code_ that happens to use the wallet
<sipa>
and a test of the wallet code itself
<achow101>
although I'm not sure if the first part that does some stuff with multisig is testing legacy wallet or just incidental
<michaelfolkson>
"a test _of non-wallet code_ that happens to use the wallet" should use MiniWallet longer term right?
<sipa>
maybe, i haven't paid attention to that
<sipa>
but probably, yes
<michaelfolkson>
But not "a test of the wallet code itself"
<achow101>
ok, I guess I will try to split it up
<sipa>
FWIW, i think a bunch of non-wallet tests of script features etc can probably be redone in feature_taproot.py
<sipa>
it has a whole framework of randomly constructing errors/transactions/scripts
<michaelfolkson>
Hmm I guess a refactor would be better but maybe someone other than achow101 could attempt it (if he has enough to do!)
<michaelfolkson>
Interested S3RK? :)
<sipa>
and feature_taproot.py itself might be portable to use miniwallet for the wallet side of things (it already mostly does its own utxo management etc)
<S3RK>
i can take on it, once i finish the coin selection improvement that I started
<S3RK>
i hope it can wait, because with my speed it might take a while :)
<sipa>
it's been there for 4 years, it won't run away
<achow101>
I have another topic if we're done with this one?
<achow101>
#topic having ScriptPubKeyMans fetch keys from a wallet keystore rather than handling keys individually
<core-meetingbot>
topic: having ScriptPubKeyMans fetch keys from a wallet keystore rather than handling keys individually
<achow101>
one of the issues with setting up multisigs and setting up new descriptors is that keys are tied to ScriptPubKeyMans, so it is difficult to get a "wallet key"
<sipa>
(judging based just on the one-line summary) that sounds like a good idea... you could think of it as the integrated "software signing device" of the wallet
<S3RK>
do you mean master key, right?
<achow101>
sipa: yes, I was thinking of something in the same model of external hardware signer but internal and software
<sipa>
i think it makes sense... there is nothing that makes private keys tied to specific SPKmans
<achow101>
S3RK: kind of
<sipa>
spkmans mostly ought to live in the "deciding what is ours" domain, not in the "what can we sign" domain
<achow101>
sipa: but "what can we sign" should align with "what is ours"
<sipa>
achow101: well, maybe- there are also external signers, watch-only wallets, multisig you can participate in
<sipa>
those are modeled by spkmans, but their signing story is kind of orthogonal
<achow101>
sipa: but all of those things would be tied to a spkman that should be in the wallet
<sipa>
right, but does it matter _which_ spkman?
<sipa>
holds the keys
<sipa>
like you can sign for something, or you can't
<achow101>
sure
<sipa>
if you'd have the same pubkey/xpub in multiple distinct descriptors, would you ever not want to sign with one, because it was linked with another descriptor?
<sipa>
you have the key, might as well use it
<achow101>
I think that's fine
<achow101>
what I want to avoid is signing for things for which we don't have a spkman for
<achow101>
e.g. if the wallet only has a wpkh(), then it shouldn't sign for multisigs even if it has the keys
<sipa>
i think that's fine; i'm not sure it's necessarily a problem if it's different
<sipa>
once an output is created it's "too late" in a sense - if you can sign for it, and it's in a transaction that you agree with, why wouldn't you?
<sipa>
of course, in practice, in order to sign you'll likely need access to an SPKMan to pull things like redeemscripts and whatnot
<achow101>
if it's in a psbt, not necessarily
<sipa>
right
<sipa>
in a psbt, i think there is no problem at all with signing whatever you can, independent of whether it's something you consider yours or not
<sipa>
but i'm also fine with "that's too hard to make work, and isn't supported"
<achow101>
it's probably easier to make that work given the way our signing code is setup
<achow101>
in any case, it seems like this change would be a good idea
<S3RK>
fwiw, I think it's good
<achow101>
it would also make adding new descriptors like tr() easier
<S3RK>
and multisig
<achow101>
any other topics
<sipa>
right, import the same xpub (maybe different branch), and signing will just work?
<S3RK>
achow101: do you plan to implement it before doing "upgrade wallet to tr" feature?
<achow101>
S3RK: probably
<S3RK>
i was about to ask how is it going. but I guess this topic was the answer
<achow101>
I'm going to try it on monday
tralfaz has joined #bitcoin-core-dev
<achow101>
S3RK: I do have a functional branch that does tr upgrade. but I don't really like that approach which is why there isn't a PR for it
freesprung has joined #bitcoin-core-dev
<S3RK>
understandable, it's hacky. well looking forward to it
<michaelfolkson>
This functional branch does what exactly?
davterra has quit [Ping timeout: 245 seconds]
<michaelfolkson>
tr(KEY) is supported already so this branch upgrades to what?
<achow101>
michaelfolkson: automagically adds a tr descriptor to a wallet
<michaelfolkson>
Ah ok
<michaelfolkson>
Without user generating it?
<achow101>
yes-ish
<achow101>
it requires knowing the spkman id for something already in the wallet and it pulls the key to use from there
<achow101>
kinda shit in terms of ux
<S3RK>
wouldn't you have the same problem when trying to pull the keys from spkman to the wallet?
<S3RK>
how do you know which key to pick? (in case different descriptors have different keys)
<achow101>
my current idea is that there would be a record in the wallet similar to the hdchain record
<achow101>
it would store the xpub of the key last used for autogenerating descriptors
<S3RK>
but that's only for the new wallets. What about the existing ones?
<achow101>
for existing wallets, we can determine that with a bit of heuristics. it would pretty much be the only key that has a descriptor for each address type
<achow101>
I think it is highly unlikely that someone would import 1 descriptor for each address type all using the same key
<S3RK>
should work 99% of the time :)
<achow101>
and if they did, I feel like they would expect that key to be used for tr too, so not really a problem :)
<achow101>
s/expect/want
<S3RK>
yes, that make sense to me
<achow101>
additionally, we should try to get this in before 23.0 because then we can say "it was experimental :shrug:"
<S3RK>
so it'll be some upgrade wallet code at load time (w/o explicit user action)?
brunoerg has quit [Quit: Client closed]
<S3RK>
I'd happily review and test this
<achow101>
yes, I don't think it will need any explicit user action
<S3RK>
sounds goot to me
<S3RK>
s/goot/good/
sipsorcery has quit [Ping timeout: 260 seconds]
sipsorcery has joined #bitcoin-core-dev
___nick___ has quit [Ping timeout: 260 seconds]
Kaizen_Kintsugi has quit [Remote host closed the connection]
Kaizen_Kintsugi has joined #bitcoin-core-dev
Kaizen_Kintsugi has quit [Ping timeout: 245 seconds]
gene has quit [Quit: gene]
kinlo has quit [Ping timeout: 245 seconds]
Talkless has joined #bitcoin-core-dev
promag has quit [Read error: Connection reset by peer]
kinlo has joined #bitcoin-core-dev
ghost43_ has joined #bitcoin-core-dev
ghost43 has quit [Ping timeout: 276 seconds]
Kaizen_Kintsugi has joined #bitcoin-core-dev
Kaizen_Kintsugi has quit [Ping timeout: 245 seconds]
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[gui] hebasto merged pull request #454: Use only Qt translation primitives in GUI code (master...2021-10-qt-use-at-translation) https://github.com/bitcoin-core/gui/pull/454
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]