< dcousens>
everytime I read IBLT, I think to myself "inverted bacon lettuce tomato"
< dcousens>
Sigh
< sipa>
LOL
< gmaxwell>
likewise, fwiw
< GitHub122>
[bitcoin] pstratem opened pull request #7064: Perform entire CWallet::TopUpKeyPool in a transaction. (master...2015-11-19-wallet-topupkeypool) https://github.com/bitcoin/bitcoin/pull/7064
< phantomcircuit>
CDB::Close abort's any active transaction, it seems like any time we're calling CDB::Close and have an active transaction in play that's an exception
< jonasschnelli>
sipa: you said when the mempool evicts a tx, the wallet threats it as conflicted. I wrote a test and "gettransaction" on a mempool-evicted tx responses confirmations:-1 but no walletconflicts (=[]). The GUI will probably threats -1 confirmations as conflicted.
< jonasschnelli>
did you mean -1 confirmation = conflicted?
< Luke-Jr>
jonasschnelli: -1 is conflicted; that it shows -1 is a bug
< Luke-Jr>
I wonder if it's safe to use the same logic as walletconflicts for that
< jonasschnelli>
Luke-Jr: But looking at CMerkleTx::GetDepthInMainChain(), it seems like that -1 just means: "not in the chain, not in the mempool".
< Luke-Jr>
jonasschnelli: at the time that was written, it implied a conflict
< Luke-Jr>
there is an assumption that anything not conflicted, will be in the mempool or chain
< jonasschnelli>
Luke-Jr: right. This assumption fails as soon as mempool can evict txes.
< Luke-Jr>
it's possible doing a more through check could harm performance
< Luke-Jr>
so I'm not sure what the best way to fix it is
< jonasschnelli>
what about "removetransaction" call for -1 confirmations txes? GUI: right click -> remove tx?
< Luke-Jr>
so only allow removing conflicted ones?
< jonasschnelli>
right.
< Luke-Jr>
sounds fine, except s/remove/hide/ at least in the low-level
< jonasschnelli>
agreed.
< gmaxwell>
gah, sipa pointed earlier that we can just check for the actual reason.
< gmaxwell>
Luke-Jr: it won't harm performance any more than what we currently do.
< Luke-Jr>
gmaxwell: oh, good, so it should be easy
< gmaxwell>
jonasschnelli: yes.
< jonasschnelli>
gmaxwell: my issue with that is, that i slowly like to reduce mempool/wallet coupling to have the possibility to detact the wallet over a SPV like connection
< jonasschnelli>
And also, how should SPV wallet deal with mempool eviction.
< gmaxwell>
This is largely unrelated to the actual mempool, it has to do more with the utxo set. Please don't overdesign right now. A SPV wallet cannot detect a non-in-wallet conflict, but we can.
< jonasschnelli>
gmaxwell: Okay. I see the point. But sipas point would require a way to check if a tx *would* be accepted in the mempool IIRC?
< jonasschnelli>
Luke-Jr : While working on this. How would RBF work in case of user interaction. Could a first feature be "altertransaction" where one could add inputs and or outputs while newfee > oldfee?
< gmaxwell>
It's more of a 'would be accepted in a block', but testing the mempool is a stronger version of that.
< gmaxwell>
For RBF fee revision the way that should work is to precalculate, with nlocktimes, transaction versions with higher fees. But do not broadcast them. This avoids having to authorize signing multiple times.
< Luke-Jr>
jonasschnelli: I would prefer it behind-the-scenes/automatic, but that may be simpler.
< jonasschnelli>
gmaxwell: huh. Sounds after a nice solution, but probably relatively complex to implement. :)
< Luke-Jr>
eg, sending another payment should probably by default be altering any outstanding unconfirmed one to add an output, but signing a normal one to broadcast in case the former confirms without it
< Luke-Jr>
hah, what I just suggested probably makes gmaxwell's look simple :D
< gmaxwell>
jonasschnelli: as far as remove, I think what would be better is 'archivetransaction' which can be done to any confirm/conflicted transaction or any unconfirmed non-conflicted at least (say) 72 hours old. (so people don't think they can cancel transactions this way)
< gmaxwell>
I think thats fairly simple because it needs basically no interface. it could be litterally a couple line loop to produce the transactions. Storing them is a bit more work.
< jonasschnelli>
Did i got this right; when the mempool evicts a wtx, the inputs automatically are release and can be used in a new transaction?
< Luke-Jr>
jonasschnelli: that's a bug because the wallet thinks it was conflicted
< jonasschnelli>
But also a feature? so "archivetransaction" is just a "visibility" thing?
< Luke-Jr>
not a feature..
< Luke-Jr>
unintended harmful behaviour <.<
< gmaxwell>
It's not a feature; it's a recently introducted bug that is very harmful.
< sipa>
jonasschnelli: hell no!
< jonasschnelli>
But if your transaction got evicted (to less fees), you can automatically create a new-one?
< Luke-Jr>
jonasschnelli: you can't deterministically respend it.. only accidentally
< gmaxwell>
it could cause you to overdraft yourself and rip people off accidentally and be unable to make it right (at least not immediately) because you no longer have enough bitcoin.
< Luke-Jr>
so if you tried to RBF this way, you could quite easily end up sendign the payment twice
< sipa>
jonasschnelli: when the mempool evicts, you specifically do not want to make the coins available again
< jonasschnelli>
sipa: so the only way to re-use the inputs would be with RBF?
< jonasschnelli>
would be/should be
< sipa>
jonasschnelli: you only do that when the reason for not being in the mempool is conflict with the blockchain
< gmaxwell>
jonasschnelli: no, we need to fix that conflict check to work correctly now that eviction is possible.
< sipa>
jonasschnelli: read the meeting log
< gmaxwell>
They should only become available again once actually conflicted (or to a RBF respend which still pays the original destination)
< sipa>
jonasschnelli: RBF is a full node side thing, not a wallet thing
< gmaxwell>
but we probably shouldn't be implementing wallet replacement behavior yet, not until after it's generally available in the network.
< gmaxwell>
Though yes, archive would be just a visibility thing.
< gmaxwell>
(and potentially memory usage and performance)
< jonasschnelli>
so we need something like this: if confirmations = -1, check if tx would be accepted to the mempool, if rejected due to non-existing inputs = conflicted
< Luke-Jr>
…no
< gmaxwell>
jonasschnelli: the whole -1 comes from checking the mempool, the specific behavior of that check just needs to be changed a bit so that it's not confused by the new behavior.
< Luke-Jr>
if the transaction isn't conflicted, then confirmations != -1
< gmaxwell>
perhaps we should also add a field on the transaction that indicates if its in the local mempool or not. (or even what its position is, now that our mempool is usefully sorted)
< sipa>
the easiest, but ugly way, is to check the return validation state code from accepttomemorypool
< gmaxwell>
(well perhaps not, since position is uncachable.; though a mempoolposition <txid> wouldn't be bad.)
< Luke-Jr>
feels like something that we'd be likely to want to change after 0.12 IMO
< Luke-Jr>
(although I wanted to give a good example reason, but I couldn't come up with one)
< gmaxwell>
Luke-Jr: what would be?
< Luke-Jr>
gmaxwell: mempool position of tx stuff
< Luke-Jr>
the exposed interface seems not necessarily obvious
< sipa>
jonasschnelli: the relevant part is the GetDepthInMainChain computation
< Luke-Jr>
jonasschnelli: the only part that needs to be touched right now, is GetDepthInMainChain
< sipa>
which looks at the mempool
< sipa>
if it isn't in the mempool, it should just return 0
< sipa>
upon first submit, and upon eviction, we should check the return value of accepttomemorypool
< sipa>
and when that indicates it's due to missing input, set a boolean flag remember in the wallet
< Luke-Jr>
sipa: well, it needs to return -1 when conflicted or we break other stuff
< Luke-Jr>
or rather, <0
< sipa>
and when GetDepthInMemoryPool would return 0 and that flag is said, only then return -1
< Luke-Jr>
IIRC the original plan was -1 if the conflicting tx was 1 block deep; -2 and so on for more blocks
< sipa>
right, that's an older idea that becomes viable again
< Luke-Jr>
but no need to do that now
< sipa>
indeed
< sipa>
Luke-Jr: we also need to be informee on eviction, with the reason for that eviction (or alternatively, resubmit upon eviction)
< Luke-Jr>
sipa: well, if we're just going to resubmit as-is, we might as well just set a priority on our own transactions so they never get evicted
< sipa>
Luke-Jr: the resubmit would fail
< sipa>
Luke-Jr: the reason to do is to find out why it fails
< Luke-Jr>
ah
< jonasschnelli>
sipa: what about informing/signaling the wallet in case of mempool eviction and set the wtx flag then (=confirmation: 0 instead of -1)? Would that be wrong?
< sipa>
jonasschnelli: read 5 lines up
< gmaxwell>
We shouldn't depend on submission or resubmission.
< sipa>
gmaxwell: hmm?
< sipa>
submission certainly
< gmaxwell>
For example, if walletbroadcast=0 we should still learn when it goes conflicted.
< jonasschnelli>
Saw that. But would that still require to evaluate the pfMissingInputs on first submitting?
< sipa>
gmaxwell: ugh
< sipa>
so we need an AcceptToMemoryPool dryrun mode?
< gmaxwell>
When the transaction is added to wallet we should check if it is conflicted. Then on every block tip change we should check all unconfirmed transactions if they are conflicted.
< sipa>
ok, i guess it's easier then to add a main function CheckForConflicgts
< sipa>
and call then if submission fails or is skipped, or we are evicted
< sipa>
and cache the result
< gmaxwell>
I don't think we need to do anything with evicted?
< sipa>
eviction also happens on block confirm, no?
< gmaxwell>
just cache until blockchain tip changes,
< sipa>
ok, that seems reasonable
< sipa>
or even cache the blockindex itself, and as long as the active one descends from it, don't reevaluate if true
< gmaxwell>
seperately it would be interesting to know if a txn was in-mempool or not, but I think thats not really cachable. :(
< sipa>
right
< gmaxwell>
sipa: yes, right once conflicted it would be nice if it were cheap, so a bunch of these don't become a performance sink in the wallet. Good call.
< gmaxwell>
non-cachability of the mempool check is perhaps not that bad though, since it would only run on transactions which are not confirmed nor conflicted.
< sipa>
gmaxwell: yes, because such a conflict check will pull in dependent utxos from the chainstate
< sipa>
so icthink we want to avoid that at all costs
< gmaxwell>
sipa: hm? To test if a transaction is in mempool we would just access the mempool by txid and check the transaction hash itself against it. Not try submitting it.
< jonasschnelli>
There is no check right now when adding a wtx to the wallet.
< sipa>
gmaxwell: well you need to check whether all inputs are available
< sipa>
gmaxwell: which means looking at the utxo set, not just the wallet
< sipa>
jonasschnelli: GetDepthInMainChain should return 0 if not conflicted
< gmaxwell>
I don't think you do, you do that via the conflicted check, which is cached.
< sipa>
gmaxwell: i'm confused; how could you detect a conflict with mempool that limited to zero?
< sipa>
it's trivial if you see some inputs spent in the utxo set
< jonasschnelli>
sipa: i think it should return 0 in that case (GetDepthInMainChainINTERNAL will result in 0), but will check
< gmaxwell>
I am saying there should be a Conflicted check that doesn't touch the viewcache, but checks the utxo set directly and is cached and keyed by blockindex.
< sipa>
gmaxwell: ok, i agree there
< gmaxwell>
If a transaction is unconfirmed and unconflicted (by the above), we could also check if the transaction is in the mempool, which is cheap.
< gmaxwell>
What I suggest wouldn't distinguish an in-mempool conflict with too-low-fee, but I think thats no big loss.
< sipa>
gmaxwell: minimal viable solution firstz place
< sipa>
grrr
< sipa>
minimal viable solution first
< gmaxwell>
ya ya. mostly its an observation that the direct test doesn't preclude also having a reasonable cheap mempoolness check.
< sipa>
what difference does in-mempoolness make? how do we report the result?
< gmaxwell>
sipa: listtransactions output, tells you if your unconfirmed tx is probably screwed. :)
< sipa>
gmaxwell: all i was saying is that jonas' conflict check shouldn't (only) look at the mempool but at the utxo set
< sipa>
in fact, i don't know if having a conflict in the mempool should matter at all for respendability... just because you accepted a different conflicting mempool tz does not mean the network will (though very likely...)
< gmaxwell>
yes, I agree, -1 should mean blockchain conflict.
< gmaxwell>
But if it does; it's useful to know that the transaction isn't in the mempool.
< sipa>
sure
< gmaxwell>
Ideally we'd give the depth of the earliest conflict, but sadly thats stupidly expensive to do. :(
< gmaxwell>
at least with how things are structured now.
< sipa>
not when there are other txouts of the txid still in the utxo set
< gmaxwell>
yes, sure but thats not reliable.
< sipa>
indeed
< CodeShark>
txindex? :)
< gmaxwell>
we could do it so long as we captured it as it flew by and recorded it in the wallet.
< gmaxwell>
in any case, the conflict depth is pretty much only interesting for small values e.g. it would be superior to not release conflicted txins for general respond until -3 (or -6) ... but no one cares about -500 vs -600
< CodeShark>
Bart is a girl? :)
< CodeShark>
gmaxwell: you mean only flush txs from mempool once they're sufficiently buried?
< sipa>
CodeShark: we're talking about the wallet
< CodeShark>
right - so the wallet needs its own mempool :)
< CodeShark>
I remember this topic a couple years ago
< gmaxwell>
it's not really a mempool thing at all.
< CodeShark>
well, it needs to keep track of its own transactions
< CodeShark>
and dependencies
< gmaxwell>
Right now the wallet will allow another transaction to reuse the non-conflicted txins from a conflicted transaction. This avoids the case when some malleated change vanishes on you, and a tx which has now been conflicted on account of it spent other coins as well, and now those coins are in limbo.
< sipa>
gmaxwell: it's really quite complicated... the question for each input is whether the corresponding is available or would become available.
< sipa>
. which means looking at the wallet, the mempool, and the utxo set
< gmaxwell>
Thats fine and all, but -1 depth is not a guarentee that the conflict will stick, there might be a reorg that unconflicts it.. so it would probably be better to not free up the inputs quite so fast.
< gmaxwell>
perhaps a cheaper approximation, is to record the height that it first hit -1 (side effect of the cache, in fact); and then make it available for respend N blocks later.
< sipa>
how do you define "hit -1"
< sipa>
all you can observe reliably is that there is an input missing
< gmaxwell>
I know, as I said 'approximation'.
< sipa>
ok
< CodeShark>
2 + 2 approximates 5 for sufficiently large values of 2
< sipa>
but that check does not output a height
< sipa>
and it depends on the mempool
< gmaxwell>
why does it depend on the mempool?
< CodeShark>
dependencies
< sipa>
because unconfirmed inputs won't be in the utxo set
< gmaxwell>
They'll be in the wallet.
< CodeShark>
no
< CodeShark>
not necessarily
< CodeShark>
they don't necessarily belong to the wallet
< sipa>
not for things that pay you
< CodeShark>
someone could construct a chain of unconfirmed transactions, only the last one pays you
< gmaxwell>
oh right, also recieved transactions can be conflicted. It's true.
< CodeShark>
for intelligent conflict detection, the wallet must be alerted when any dependency is conflicted
< gmaxwell>
sorry, had slipped into thinking only in terms of sent because I was thinking about the free to reuse question.
< sipa>
received transactions matter even more, because they're the only ones that give spendable outputs
< gmaxwell>
sipa: 0_o. change.
< sipa>
oh, right
< gmaxwell>
(googlyeyes simply because basically all conflict originated problems are related to change; since change is the only thing we'll spend with few/no confirms!)
< Luke-Jr>
gmaxwell: just send all your change to sipa and that problem goes away
< * sipa>
approves
< gmaxwell>
new wallet feature: eliminates all malleability problems by sending your coins to sipa.
< Luke-Jr>
:D
< sipa>
gmaxwell: now.i wonder what googly eyes are...
< Luke-Jr>
sipa: "0_o"
< CodeShark>
basically, there currently is no safe way to spend an unconfirmed output ;)
< CodeShark>
even your own
< CodeShark>
for some malleability attacks about the best you can do is sign all likely cases
< CodeShark>
and have the txs ready to broadcast
< wumpus>
there's an option in bitcoin core's wallet to not spend any unconfirmed outputs
< CodeShark>
but then your funds potentially get tied up for a while
< wumpus>
it is only not the default because that was deemed too big a change from previous behavior
< CodeShark>
and it's also impractical in many scenarios
< gmaxwell>
CodeShark: well it'll be pratically pretty safe soon.
< sipa>
gmaxwell: the wallet does get informed about all blockchain transactions, so i think we can just use that to detect conflicts
< CodeShark>
yes, indeed - I'm very happy about that, gmaxwell
< gmaxwell>
The don't spend unconfirmed change thing is pretty obnoxious unless you understand bitcoins operation very well and intentionally create extra outputs.
< wumpus>
anything involving zero confirmation is risky
< sipa>
we should be creating extra outputs anyway for amount privacy
< CodeShark>
about the best way to mitigate having funds tied up for a while is to create denominated outputs up front
< CodeShark>
which is ugly in terms of block chain bloat
< wumpus>
absolutely, better output management could avoid having *all* your funds tied up
< wumpus>
e.g. by having multiple change outputs
< gmaxwell>
During the last malleability attacks I walked a bitcoin ATM operator through doing that. Hard part was explaining the need, once they got it the actual implementation was easy. (well also the wallet they were using to fill their ATMs ..... had no sendmany support. :( )
< gmaxwell>
sipa: yea, did I ever tell you my thought? if the change would be 'large' flip a coin and either split the change 50/50, or make one change equal to the payment and the other the remainder.
< sipa>
gmaxwell: yup, that'd be useful
< sipa>
gmaxwell: except when the payment is a nice round number
< sipa>
gmaxwell: then you probably always want to make the change identical to it (or a round fraction/muktiple)
< CodeShark>
denominating outputs in powers of 2 and limiting precision can help
< CodeShark>
sometimes having the extra precision means adding dust outputs :)
< gmaxwell>
sipa: well the decision could be biased, but the basic idea is reasonable.
< sipa>
gmaxwell: agree
< sipa>
gmaxwell: if conflicting wallet transactions with change
< sipa>
can be accepted without conflicting
< sipa>
then inthink our balance calc will be off
< CodeShark>
a scheme that avoids the need for change outputs without bloating the blockchain would fix a bunch of problems
< CodeShark>
I've also had issues with asynchronous signing
< CodeShark>
perhaps some transaction is rejected by the authorization layer and never gets signed...which ties up the outputs
< CodeShark>
which means it ties up any attempts to sign dependencies
< sipa>
oh! we should only count unconfirmed transactions (from ourselves) if they are in the mempool
< wumpus>
sounds sensible sipa
< sipa>
but only mark them conflicted/respendable when they have a conflict in the chain
< wumpus>
right
< CodeShark>
more sensible than what we currently do...but still has issues
< CodeShark>
I guess the UI could indicate that certain transactions have failed
< CodeShark>
and prompt the user to resend
< CodeShark>
or automatically resign and resend if the keys are unencrypted
< CodeShark>
although that might open up security holes
< sipa>
CodeShark: the plan was to have a function to mark a tx as failed
< sipa>
manually
< CodeShark>
well, other than the usability complications, it technically is consistent behavior
< CodeShark>
and arguably less problematic than having a zero fee transaction in limbo for a long time :)
< tulip>
is there an opportunity in all this for a user to "remove" a transaction, make an alternative with different outputs, and both confirm?
< gmaxwell>
I'm really not keen on requiring the manual intervention, I can see this resulting in lost funds.
< gmaxwell>
Consider an automated user, ends up with some conflicted transactions. Further spending gets rejected with insufficient funds, they think they're out of bitcoins and delete the wallet-- but they could have had a large amount remaining.
< gmaxwell>
tulip: you mean spending different outputs (different inputs)?
< tulip>
yes.
< gmaxwell>
That generally exists, even now. Though they don't have a remove button which they might misunderstand to abort the transaction.
< gmaxwell>
We can harden against that in the GUI by making the wallet warn you when you paid and address you've paid before. (something suggested before, because it can also avoid copied-the-wrong-line and double paid problems)
< CodeShark>
I've had to directly deal with this issue in production before...and had to enable deleting transactions that have already propagated...and yes, it's a mess to keep track of stuff
< gmaxwell>
well we have zapwallettx to knock out any conflicted/unconfirmeds from the wallet, tech support via napalm.
< CodeShark>
lol
< sipa>
napalm is cool, when frozen
< gmaxwell>
^ man, I want a bot that produces comments like that.
< CodeShark>
gmaxwell: I went ahead and added an API call to delete transactions - initially it would only allow deleting transactions that have not yet propagated (i.e. missing signatures)
< gmaxwell>
in any case a dialog that says "You've paid X before, are you sure?" then shows the most recent payments to X. would likely help a lot there for gui users.
< CodeShark>
but I had to eventually allow deleting even propagated transactions for malleability attacks and transactions that just won't confirm (low fees)
< CodeShark>
and for any sort of high volume operation it's a nightmare
< gmaxwell>
CodeShark: have you yet had someone have something deleted make it through and be surprised?
< CodeShark>
fortunately no terrible losses have occured...but it has required extra careful manual intervention
< gmaxwell>
I once found a mtgox transaction that was created almost a year before but managed to never make it into the chain (maybe wasn't relayed well), and was still valid.
< CodeShark>
and ultimately, being extra careful also means creating a bottleneck and greatly reducing transaction processing volume (or grinding to a halt)...which some consider bad for business
< GitHub86>
[bitcoin] paveljanik opened pull request #7066: [Trivial,Doc] Add missing "blocktime" description to listtransactions help, fix formatting. (master...listtransactions_blocktime) https://github.com/bitcoin/bitcoin/pull/7066
< CodeShark>
gmaxwell: where did you find it? :)
< gmaxwell>
For people I've helped with this, they did batch correction, where basically they shut down operations temporarily, waited for all outstanding to confirm, extracted all the transactions, checked all the conflicted outputs against the blockchain to figure out who hadn't been paid yet. zapped the wallet. Redid the missing payments with a transaction that spend all their coins.
< gmaxwell>
(and also split up their wallet into new outputs, so they could reasonably run with spending unconfirmed change off)
< kanzure>
huh, what are the chances of an ancient mtgox transaction still being valid? i mean, that would require a lot of utxos..
< gmaxwell>
mtgox intentional split their wallet into tiny utxo.
< gmaxwell>
My memory is a bit fuzzy, I think what the sequence of events was is that when mtgox was imploding I was trying to determine all the addresses that were theirs. And someone had been logging their api output that listed all their transactions.. and I was going through all that to find every address they every claimed to sign for.
< gmaxwell>
And noticed that one of the old ones had an unspent input.
< kanzure>
smaller than dust?
< gmaxwell>
no, not that small. The code that did that is public in that leaked php I think.
< tulip>
the repo with all of the logged transactions is on github, somewhere.
< kanzure>
tulip: ah didn't know, thanks
< kanzure>
v. nice to have this historical data
< gmaxwell>
I think the algorithim was something like if a txout is >threshold (10btc?) split, with a ration that is some exponential distribution, with 50/50 the most common, or something like that. (could be my imagination)
< GitHub15>
[bitcoin] jonasschnelli opened pull request #7067: [Wallet] improve detection of conflicted transactions (master...2015/11/mempool_wallet) https://github.com/bitcoin/bitcoin/pull/7067
< midnightmagic>
gmaxwell: progress projects a 6-9 day reindex. do you want me to just let you know when it gets to head, or is that fact it's churning right now good enough for you? commitid: a1bfca80521ee99d70bc19a797484275d84e136f
< gmaxwell>
midnightmagic: that its churning is good information!
< midnightmagic>
gmaxwell: okie doke.
< GitHub146>
[bitcoin] jonasschnelli opened pull request #7068: [RPC-Tests] add simple way to run rpc test over QT clients (master...2015/11/rpc_tests_qt) https://github.com/bitcoin/bitcoin/pull/7068
< gmaxwell>
[OT] Sad news, ITU announces that they've decided to not stop creating leapseconds for now, and spend 8 more years researching the impact.
< morcos>
jtimon: i saw that. i like the idea, but not exactly that. I think it is usefull for TrimToSize still take an argument, and TrimToSize should then set the internal attribute of the mempool letting it know what size it has been set to.
< morcos>
That way the policy logic on what size the mempool should be trimmed to is outside the mempool. GetMinFee could access that internal attribute in the same way you have it
< morcos>
I don't love calling that from txmempool.cpp, since I'd like that pass through function to go away, and the policy estimator to be called directly from the wallet I htink?
< morcos>
My concern about that is someone needs to know to ask the mempool for its min fee if you want to estimate fees? Where should that logic live. IN my opinion not in the mempool.
< morcos>
So it doesn't seem unreasonable to me that the policyestimator is going to need to ask the mempool questions. what questions it needs to ask and what it does with them, seem logic that could live in the estimator itself. and therefore you have to pass in a pool, but i don't know...
< morcos>
In any case, I'm out of town for the next week+ so I'm not really going to have much time to modify it myself. However if we end up with some changes that we think are good, I'm happy for you or sdaftuar to incorporate them
< GitHub11>
[bitcoin] MarcoFalke opened pull request #7070: Move hardcoded maxFeeRate out of mempool (master...MarcoFalke-2015-feeRateRefactor) https://github.com/bitcoin/bitcoin/pull/7070
< phantomcircuit>
jgarzik, in CDB::Close activeTxn is closed if it's open, this is legacy code from satoshi, but your'e the last to have touched it
< phantomcircuit>
any idea why that's there?
< jgarzik>
phantomcircuit, speed IIRC. We fiddled a lot with that during IBD tuning days. IBD was very sensitive to when things were flushed
< jgarzik>
phantomcircuit, without special logic at close, shutdown would pause for a long time
< phantomcircuit>
jgarzik, i can see that support for nested transactions was dropped but i dont see what this has to do with flush logic
< jgarzik>
phantomcircuit, state mgmt -- during IBD you held the transaction open because you might get more blocks
< phantomcircuit>
jgarzik, hmm
< jgarzik>
phantomcircuit, since you might be in the middle of IBD during shutdown, you might have a transaction open on purpose
< jgarzik>
phantomcircuit, keeping the transaction open is poor man's write batching in the face of async "block" msgs
< phantomcircuit>
jgarzik, the wallet keeps a transaction open?
< phantomcircuit>
i can see leveldb doing that but why would the wallet need to do that
< jgarzik>
phantomcircuit, BDB transaction
< phantomcircuit>
ohhh it's from when we were using bdb for the chain state
< jgarzik>
phantomcircuit, nod -- I'm talking about logic put in place during the days when we stored chain state in there
< phantomcircuit>
ok so that can be safely changed to be an assert
< jgarzik>
phantomcircuit, wallet should never do that
< jgarzik>
indeed
< phantomcircuit>
jgarzik, great that will make a bunch of stuff much easier for me
< GitHub82>
[bitcoin] pstratem opened pull request #7071: Wallet: Replace TxnAbort with assert. (master...2015-11-20-wallet-activetxn) https://github.com/bitcoin/bitcoin/pull/7071