< jtimon>
\open_poll Would you (A) or (B) #9176 once and for all? It's never costing rebase time but potentially review time just by being open: A: CLOSE, B: MERGE
< gribble>
https://github.com/bitcoin/bitcoin/issues/9176 | Globals: Pass Consensus::Params through CBlockTreeDB::LoadBlockIndexGuts() by jtimon · Pull Request #9176 · bitcoin/bitcoin · GitHub
< phantomcircuit>
gmaxwell, i think it's just wrong?
< sipa>
it looks to me he's removing only unused code
< sipa>
which it shouldn't
< sipa>
be
< sipa>
but maybe i miss something
< gmaxwell>
sipa: hm? it's hooked up to the Broadcast signal, which is invoked from SendMessages.
< sipa>
oh
< gmaxwell>
have I said before that I do not slots and signals they and overhead and _obscure control flow_? :P
< sipa>
gmaxwell: well i'm confused by ryanofsky thinks it's unused
< gmaxwell>
maybe there is some pattern used in the codebase which makes the control flow less clear to him? :P (I thought we had tests though for wallet retransmission, but seems the tests are passing. :( )
< jonasschnelli>
Anyone up for a quick wallet PR review with great positive performance impacts: #10251
< gribble>
https://github.com/bitcoin/bitcoin/issues/10251 | Add balances cache / GUI: use a signal instead of a poll thread by jonasschnelli · Pull Request #10251 · bitcoin/bitcoin · GitHub
< ryanofsky>
re: "well i'm confused by ryanofsky thinks it's unused" just a mistake, my brain wasn't working, and the naming for that signal doesn't follow the normal pattern
< bitcoin-git>
[bitcoin] morcos opened pull request #10586: More economical fee estimates for opt-in-RBF transactions (master...aggressiveEstimates) https://github.com/bitcoin/bitcoin/pull/10586
< morcos>
sipa: Ruling requested
< morcos>
What is the style guide for an rpc named argument?
< morcos>
conf_target?
< morcos>
we already have a confTarget inside the options for bumpfee, but its inside the options, and it seems most named argumetns aren't camelCase
< sipa>
from developer-notes.md, under RPC:
< sipa>
Argument naming: use snake case fee_delta (and not, e.g. camel case feeDelta)
< sipa>
Rationale: Consistency with existing interface.
< bitcoin-git>
[bitcoin] practicalswift opened pull request #10587: Net: Fix resource leak in ReadBinaryFile(...) (master...fopen-not-followed-by-fclose-in-all-states-of-the-universe) https://github.com/bitcoin/bitcoin/pull/10587
< morcos>
sipa: oops, sorry i didn't read
< cfields>
jtimon: ping
< jtimon>
cfields: pong
< cfields>
jtimon: I'm confused. At one point, you had a version of the reverse_iterator that compiled, but failed tests. no?
< jtimon>
yes, and then I followed your advice for the prevector tests, which was the part that didn't compile unless I commented those lines (but if I did comment them, then the tests failed as they should)
< cfields>
jtimon: can you point me to a failing revision?
< cfields>
jtimon: oooooh
< cfields>
jtimon: the tests failed because some things were commented out, not because of new breakage?
< jtimon>
right the breakage was a compile error
< jtimon>
sorry, I shouldn't have commented the lines, and should had left the compile error there, I just wanted to see what else would fail
< cfields>
jtimon: ok, i misunderstood, then
< cfields>
jtimon: i wrote some quick tests locally, and everything passed. So I'm not so concerned about the scope issue anymore
< jtimon>
cfields: oh, thanks! I tried to do the reverse iterator more const, but not the prevector!
< bitcoin-git>
[bitcoin] morcos opened pull request #10588: Note preexisting bug in display of fee calculation in coin control (0.14...notebug) https://github.com/bitcoin/bitcoin/pull/10588
< instagibbs>
anyone know the reasoning for the "keyword definition" stuff in importmulti.py? A couple of those strings don't even occur, and none of the variables are ever used.
< bitcoin-git>
[bitcoin] morcos opened pull request #10589: Add RPC options for RBF, confirmation target and conservative fee estimates (master...rpcestimatechoice) https://github.com/bitcoin/bitcoin/pull/10589
< instagibbs>
morcos, really wish we could get some basic effective value logic in so we could easily improve bumpfee as well.
< morcos>
how does that help with improving bumpfee?
< instagibbs>
Well, I guess we could just lamely loop and keep increasing until it grabs enough effective value...
< instagibbs>
the sickness spreading :P
< instagibbs>
right now bumpfee must have a change output to be successful
< morcos>
instagibbs: yes but i don't understand how thats related to the effective value logic?
< instagibbs>
if we had effective value logic, it would be easier? Sorry I'm missing what you're missing.
< morcos>
i thought the reason bumpfee had to have change had something to do with the complication of adding new inputs which may mean there are multiple txs being replaced
< instagibbs>
ah, maybe that's a reason
< morcos>
i dont' think the requirement that we had change to reduce in order to bump fee had anything to do with our stupid coin selection
< morcos>
in any case, i'm all for improving coin selection.
< instagibbs>
why would adding confirmed inputs replace multiple txs?
< instagibbs>
maybe I should go hunt down the bumpfee PR, find the undocumented assumptions
< morcos>
but limited cycles.. i owe sipa and bluematt tons of review too, but i'm about to be away from work for a bit, so i just wanted to push out the fee changes i think we need for 0.15
< instagibbs>
Understood
< morcos>
instagibbs: i don't remember exactly now, but i thin kthe idea was just to limit it to the simplest possible case for starters
< morcos>
not that we can't do other things , but just to get the first round in
< instagibbs>
I totally agree, which is why I was thinking about coin selection in that respect.
< instagibbs>
anyways I'll revisit it post-0.15
< sipa>
instagibbs, morcos: achow101 and i are looking at murch's branch&bound algorithm for coin selection
< instagibbs>
great. We might not care about exact matches when hitting `bumpfee`, since we may want followup change
< instagibbs>
without linking a bunch of inputs together greedily
< murch>
@sipa: What are you guys doing this evening?
< murch>
I assume you have found the branch in my github repo?
< bitcoin-git>
[bitcoin] luke-jr closed pull request #10512: Rework same-chain from abusing DoS banning, to explicit checks (master...samechain_rework) https://github.com/bitcoin/bitcoin/pull/10512
< bitcoin-git>
[bitcoin] luke-jr opened pull request #10593: Relax punishment for peers relaying invalid blocks and headers (master...relax_invblk_punishment) https://github.com/bitcoin/bitcoin/pull/10593
< gmaxwell>
instagibbs: I believe the reason for fixing the inputs was just to simplify the first implementation.
< bitcoin-git>
[bitcoin] luke-jr opened pull request #10594: Bugfix: net: Apply whitelisting criteria to outgoing connections (master...whitelist_outgoing) https://github.com/bitcoin/bitcoin/pull/10594
< gmaxwell>
Also, it's preferable to not use extra inputs if you can avoid it, because you'll burn down your unspent outputs available for other transactions. (keep in mind you really don't want to spend the bump change, since you can't be sure which version will confirm)