< fanquake> So here's a new? annoying GitHub behaviour.
< fanquake> So #101 got auto-closed when it was merged.
< gribble> https://github.com/bitcoin/bitcoin/issues/101 | Shy clients by gavinandresen · Pull Request #101 · bitcoin/bitcoin · GitHub
< fanquake> 101 was then re-opened, because it needed further fixing.
< fanquake> I fetched upstream, merged it into my bitcoin fork, then pushed that back to GitHub (fanquake/bitcoin).
< fanquake> Github then re-closed 101...
< fanquake> I don't think I've ever seen that happen before?
< fanquake> Does this happen when anyone (with merge permissions, or maybe not even that?) just does the same?
< fanquake> Seems like something we should have definitely noticed in the past if so
< sipa> i also think we don't have much experience with such "fixes: <issue on other repo>" tags
< fanquake> It seems quite annoying if GitHub will close issues in the bitcoin-core/gui repo, when I, or potentially anyone else, push to their own repos (fanquake/bitcoin)
< sipa> you should try to see if it works for a repo you don't have write access to ;)
< fanquake> I will save debugging GitHub for this afternoon
< bitcoin-git> [gui] Bosch-0 opened pull request #199: Added updated bitcoin PNG / SVG icon (master...bitcoin_icon) https://github.com/bitcoin-core/gui/pull/199
< dhruvm> (doesn't address the weird behavior with your own fork though)
< gwillen> requires you have permission to push to the mentioned repository
< sipa> gwillen: way to shatter our dreams about having discovered an amazing oversight in github's security :(
< gwillen> don't worry, I'm sure you'll find another one pretty soon
< bitcoin-git> [bitcoin] fanquake opened pull request #21016: refactor: remove boost::thread_group usage (master...remove_boost_threadgroup) https://github.com/bitcoin/bitcoin/pull/21016
< vasild> MarcoFalke: wrt https://github.com/jnewbery/bitcoin/pull/18#issuecomment-768216999: -use_value_profile=1 does not make much difference, if any
< vasild> I added -max_len=1048576 too in an attempt to make it provide more fuzz data
< vasild> ConsumeNetAddr() generates an unroutable address most of the time
< vasild> I will see how to make ConsumeRoutableNetAddr() (retrying ConsumeNetAddr() until it produces a routable address is a bad idea :)
< bitcoin-git> [bitcoin] MarcoFalke pushed 5 commits to master: https://github.com/bitcoin/bitcoin/compare/e130ff38c91a...11d3b5833671
< bitcoin-git> bitcoin/master 784a278 Jon Atack: doc: update -onlynet help in src/init.cpp
< bitcoin-git> bitcoin/master dfc4ce1 saibato: doc: update -proxy, -onion and -onlynet info in tor.md
< bitcoin-git> bitcoin/master 9af99b6 Jon Atack: doc: update/improve automatic tor section of tor.md
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #20757: doc: tor.md and -onlynet help updates (master...tor-md-doc-updates) https://github.com/bitcoin/bitcoin/pull/20757
< bitcoin-git> [bitcoin] laanwj pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/11d3b5833671...15a9df070615
< bitcoin-git> bitcoin/master a6739cc Wladimir J. van der Laan: rpc: Add specific error code for "wallet already loaded"
< bitcoin-git> bitcoin/master 15a9df0 Wladimir J. van der Laan: Merge #20964: rpc: Add specific error code for "wallet already loaded"
< bitcoin-git> [bitcoin] laanwj merged pull request #20964: rpc: Add specific error code for "wallet already loaded" (master...2021-01-wallet-already-loaded-rpc) https://github.com/bitcoin/bitcoin/pull/20964
< bitcoin-git> [bitcoin] laanwj closed pull request #20957: doc: Update tor.md for notes on how to preserve v2 urls (with a not recommended note) (master...tor-controlport-docs) https://github.com/bitcoin/bitcoin/pull/20957
< semtexzv> Hello there, I'm reading through the eltoo paper + BIT 118, and had an idea.
< semtexzv> the SIGHASH_NOINPUT exposes an address to weird replay attacks. However, it's not needed for the eltoo. Eltoo channels know previous states, from which they could want to spend.
< semtexzv> So, how about encoding a SET of possible inputs to a transaction by encoding it as a merkle tree, with the usage of signature revealing a path within the merkle tree ?
< semtexzv> Basically, a new sighash type, encoded as <sig> SIGHASH_SELECT <merkle_root> <merkle_path> , with the merkle root part included in the signature input.
< semtexzv> That would basically allow to use constrained set of UTXOs, at the cost of signature effectiveness ( log(n) cost ), which is only used in the resolution branch, not the normal settlement.
< shesek> it would be useful if listtransactions/listsinceblock reported which of the conflicted mempool wallet transactions is considered to be the 'active' one and which are considered to be double-spent
< sipa> if it's in the mempool, it's considered active ;)
< shesek> currently, when I see a wallet transactions with confirmations == 0 and a non-empty walletconflicts, I have to issue an additional `getmempoolentry` rpc call to determine whether the transaction is 'active' or not
< shesek> right ^^
< shesek> but it would be nicer if this was more readily available in listtransactions
< sipa> ah
< shesek> and more efficient, less rpc calls
< shesek> btw, is there a more common term to describe mempool transactions that are 'active' from the PoV of your node?
< luke-jr> "in mempool"?
< luke-jr> shesek: why do you think this distinction matters? <.<
< shesek> hmm, yeah, I guess so
< luke-jr> ie, what is the use case?
< shesek> what do you mean why it matters? the 'active' transaction should be shown to the user in the wallet history and can be eligible for coin selection, the others should not
< luke-jr> why not?
< luke-jr> unconfirmed is unconfirmed
< gwillen> would we ever do coin selection from a zero-conf transaction we know is conflicted? (I'm curious what it even means for one to be active. Would this be for RBF, where one is expected to confirm much more surely than the other?)
< shesek> gwillen, I'm running into this in the context of rbf, yes. for example, following a fee bump, the wallet could choose to spend the higher-fee change output, but not the lower-fee one
< shesek> it also shouldn't show the lower fee one in the wallet history, or show it with some different graphic, or behind an 'advanced' screen
< luke-jr> you have higher- and lower- backward there I think?
< gwillen> *nod*, makes sense. Given that we already allow coin selection from 0-conf, it seems like you'd want this.
< luke-jr> gwillen: I would think you want to spend any conflicting change from all possible inputs concurrently, if at all
< shesek> luke-jr, hmm, I think I got it right? I was referring to higher/lower fee, not the value of the change output (which would be backwards)
< luke-jr> shesek: but you're more likely to use change if it's higher (therefore lower fee)
< gwillen> I'm curious what the current wallet behavior is here.
< luke-jr> gwillen: I'd guess a mess :P
< luke-jr> one of those reasons we should remain 0.x ;)
< shesek> I noticed that when calling `gettransaction` on the 'non active' mempool tx, the `details` object is empty. which suggests to me that bitcoind is probably ignoring these somehow
< shesek> this seems somewhat inconsistent, listtransactions/listsinceblock are basically a flattened concatenated list with the `details` from all the wallet transactions, I would expect transactions that report an empty `details` object in `gettransaction` not to show up on `listtransactions` at all
< shesek> in other words, wallet transactions with N entries in `details` show up N times in `listtransactions`, except for N==0
< sipa> shesek: that sounds inconsistent indeed
< bitcoin-git> [gui] laanwj merged pull request #180: Peer details: connection type follow-ups (master...peer-details-connection-type-followups) https://github.com/bitcoin-core/gui/pull/180
< bitcoin-git> [bitcoin] laanwj pushed 4 commits to master: https://github.com/bitcoin/bitcoin/compare/15a9df070615...d1ddead09a58
< bitcoin-git> bitcoin/master 4f09615 Jon Atack: gui: return inbound {full, block} relay type in peer details
< bitcoin-git> bitcoin/master f3153dc Jon Atack: gui: improve markup handling of connection type tooltip
< bitcoin-git> bitcoin/master 79a2576 Jon Atack: doc: update ConnectionType Doxygen documentation
< shesek> for my use case, I would prefer they do show up in both gettransaction's `details` field and in listtransactions/listsinceblock, but with a flag that indicates their status. similarly to when its conflicted with a confirmed transactions
< sipa> i think at some point confirmations=-1 meant that a transaction wasn't in the mempool
< shesek> this makes it easier to sync the wallet state using listsinceblock/listtransactions
< sipa> but later (still many years ago) that was changed so that -N confirmations means "a conflict to this tx has N confirmations"
< shesek> sipa, hmm, I see. this wouldn't be compatible with the current definition of negative confirmations
< shesek> right
< S3RK> luke-jr wrt https://github.com/bitcoin/bitcoin/pull/20226#issuecomment-768392164 I've pushed updated test with my comment so it won't fail anymore. Sorry it wasn't clear from my original comment
< S3RK> achow101 could you expalin what do you mean by "to not make tests dependent on --descriptors"
< achow101> S3RK: the idea is that all tests do the necessary things when ran without command line arguments. so tests that do both legacy and descriptor wallets, not specifying the argument runs both sets of tests. and for tests for a specific wallet type, running without arguments just does the one wallet type it is supposed to test
< achow101> then if you specify a specific type and the test doesn't support that wallet type, it skips
< achow101> #20892 is the relevant pr for that
< gribble> https://github.com/bitcoin/bitcoin/issues/20892 | tests: Run both descriptor and legacy tests within a single test invocation by achow101 · Pull Request #20892 · bitcoin/bitcoin · GitHub
< S3RK> interresting, i'll check out this PR
< S3RK> achow101 wiling to reACK if I apply fixes suggested by jonatack?
< S3RK> s/wiling/willing/
< achow101> sure
< jonatack> of course he is *ducks*
< jonatack> S3RK: but they're no big deal
< shesek> I filed an issue for conflicted wallet transactions: https://github.com/bitcoin/bitcoin/issues/21018
< shesek> wasn't sure if the inconsistency qualifies as a bug or not, ended up submitting the issue as a feature request
< luke-jr> achow101: S3RK: I was thinking default --descriptors on when it's required, but achow101's idea seems better
< luke-jr> although with that, will test_runner still produce a SKIP message when you build w/o sqlite?
< achow101> luke-jr: the current implementation won't produce a skip message. but maybe it could be a "partial skip" message?
< luke-jr> achow101: with that patch I posted it would
< luke-jr> feature_backwards_compatibility.py --descriptors | ○ Skipped | 19 s
< achow101> luke-jr: the idea is to not have to have --descriptors in test_runner commands
< luke-jr> achow101: right, I'm just saying it'd be nice to not lose the Skipped lines in the process
< achow101> I suppose
< luke-jr> I guess it's possible to have the default run both, and then specify both --legacy and --descriptors in test_runner..