< phantomcircuit>
morcos, my huge sigcache is mostly fine
< morcos>
phantomcircuit: i'm running with 1M now and its still noticeably slower (than infinite)
< phantomcircuit>
changing the max arena 100% fixed the problem without rewritting a bunch of stuff too :)
< phantomcircuit>
hmm it really shouldn't be
< morcos>
well i'm chaning the other stuff anyway :)
< morcos>
well noticeably slower i mean just that... say instead of taking 30us to verify a txin on average it takes 40us
< morcos>
but without a cache at all of course its 10x higher
< morcos>
its the process of filling up your 300M mempool for the first time that makes it slow... i guess it should improve once its full and the mempool gets a minfee
< morcos>
scarily fast filling up to 300M from starting from scratch. like 45mins
< phantomcircuit>
morcos, without the limiting
< phantomcircuit>
im at about 2GB now
< phantomcircuit>
3.168GB
< morcos>
i'm still disturbed by how this same backlog of txs is so readily rerelayed
< gmaxwell>
esp since the network behavior is supposted to not relay transactions once they've already been relayed. :(
< gmaxwell>
unfortunately there seem to be some abusive nodes out there that behave strangely, constantly resending transactions and such.
< morcos>
yeah i noticed that, i was wondering if they were nodes who considerd these wallet tx's or if was specifically designed to retransmit them
< morcos>
ok modulo the final TestBlockValidity check, i brought the time of CreateNewBlock from a bit over 1 second to under 10 ms.
< gmaxwell>
the ones I've caught doing it with an instrumented node appeared to regurgitate their whole mempool every block.
< morcos>
unfortunately i wouldn't feel good about dumping that check.
< morcos>
that also doesn't include adding any priority transactions. i can add that back in, but i think it'll be slow
< morcos>
gmaxwell: oh really?? thats bad
< midnightmagic>
fwiw, gmaxwell recall you wanted me to check and see whether any of those nodes were resending me tx dupes: across the entire time I was monitoring, none of them did that I could detect while parsing the debug.log.
< midnightmagic>
I don't think I actually got back to you about that.
< midnightmagic>
gmaxwell: What's the periodicity of the resends?
< gmaxwell>
midnightmagic: at least some were doing this after every block.
< gmaxwell>
e.g. a bogus assumption that any transaction known would make it in a block, and if it didn't you must have just 'lost' it.
< gmaxwell>
or the like.
< midnightmagic>
I wonder if some services are attempting to "help" the network by propagating tx in order to guarantee reach for nodes that have rebooted or something.
< CodeShark>
sending as in sending the actual transaction? or just an inv?
< phantomcircuit>
CodeShark, same thing for this purpose
< phantomcircuit>
zombie transactions
< CodeShark>
for what purpose? I would think sending the mempool (as in the actual txs, not just the inv) is MUCH worse for bandwidth
< morcos>
would anybody else find useful a feature which split the minimum fee for acceptance to your mempool and the min fee for relay?
< morcos>
often during this testing, i either hack up my node to not relay, or i feel bad about the fact that i'm helping propogate these low fee txs all over again
< CodeShark>
I would find really useful a feature that would allow you to get paid for running a validator/relay node :p
< morcos>
you get paid in food for your soul
< CodeShark>
lol
< sipa>
CodeShark: i think that screws incentives completely
< CodeShark>
sipa: howso?
< sipa>
CodeShark: if you can get paid to run a node, you can equally get paid to run a compromised node
< CodeShark>
presumably you'd have to actually perform the function sufficiently well
< sipa>
you can't prove that you are
< CodeShark>
well, relay might be more easily amenable to micropayments - validation is a little trickier
< CodeShark>
but I think ultimately doable...not sure how, but my gut tells me at the very least some efficient probabilistic checks are possible
< CodeShark>
without having to go to SNARKs
< sipa>
their validation is unobservable... it's a feature they provide to their users, not to you
< CodeShark>
it's a feature they provide to the network
< sipa>
no
< sipa>
relay is what they provide to the network
< sipa>
and you can't observe that they validate before relay
< sipa>
1) they may just be connected to honest nodes them selves, so they'll only relay valid blocks through that
< sipa>
2) past performance is not an indicator for future success. One awesome way to sybil attack the network is to spin up many totally honest nodes, and then suddenly turn evil once you notice a sufficient portion of the network depends on you.
< sipa>
relay/ibd is a dumb service they offer to the network - which hopefully verifies what you tell them enough to not need to trust you
< sipa>
the power of validation is only through knowing that the person running it has economic activity depending on it
< CodeShark>
so I guess if the network is to subsidize it it would have to either require miners to be doing validation somehow...or to incentivize invalidation
< CodeShark>
in any case, if we could just take care of incentives for relay for now I'd be happy :)
< CodeShark>
that would go a long ways towards solving the mempool issues
< CodeShark>
providing SPV proofs could be incentivized...and perhaps nodes that can provide SPV proofs would also have to perform full validation
< CodeShark>
or it would be in their interest to, I suppose
< phantomcircuit>
morcos, i always return early in relaytransaction when messing with mempool stuff
< phantomcircuit>
ha i can detect which peers are master by the order of their responses to the mempool command
< morcos>
phantomcircuit: is this a new node? not the 3GB one? its not surprising. most of these spam txs have 100txins, so you'd assume about 100 to 1 ration of txs recieved to setValid size
< phantomcircuit>
gmaxwell, sigcache vs mempool size
< phantomcircuit>
morcos, it's currently talking upto being full
< phantomcircuit>
takes ages
< phantomcircuit>
ok so 1,000,000 sigcache ~= 300MiB mempool
< phantomcircuit>
we should definitely try to switch to a single parameter "-memlimit"
< phantomcircuit>
and then split it out as percentages
< jcorgan>
in my experience that is almost certainly an out-of-memory problem
< dcousens>
jcorgan: interesting, I am trying to compile it on a device with only 1GB
< jcorgan>
look in dmesg for linux OOM killer action
< dcousens>
yeah I did, you were right
< gmaxwell>
add swap. There are some options you can give g++ which will drastically reduce memory usage... but I have no idea what they are. :)
< dcousens>
gmaxwell: easier to just put in another stick atm
< dcousens>
poop
< dcousens>
2GB still dies lol
< dcousens>
time to look up those g++ options...
< dcousens>
or swap... fk it I'll add swap
< dcousens>
hmmm, dmesg not showing OOM this time
< dcousens>
but same error
< wumpus>
2GB should certainly be enough. See e.g. https://github.com/bitcoin/bitcoin/issues/6658 , worst contender main.cpp uses 1.2MB while compiling (on gcc 4.8). Adding swap may help though, linux' memory management works better if swap is enabled even if you don't need the extra memory
< wumpus>
if you want to build bitcoind for a smaller device I'd recommend cross compiling though, it's easy with the depends system
< dcousens>
I ended up `git clean -xdf`, re-configuring/autconf and it got past that second hurdle
< dcousens>
Guessing it configured something different when I only had 1GB?
< wumpus>
it doesn't
< gmaxwell>
wumpus: some people are managing to run with no overcommit _and_ no swap; ... dunno how anything at all works for them.
< dcousens>
Then I have no idea, except that it worked this time, but again, no OOM from dmesg so not sure
< wumpus>
but it may be possible for gcc, when it crashes, to generate a poop file that will crash it later
< dcousens>
gmaxwell: eh, on my own home nodes I always ran without swap
< dcousens>
but, they always had a bit more hardware to play with
< gmaxwell>
dcousens: note that I said both no swap and no overcommit.
< wumpus>
gmaxwell: I guess if the workload is very predictable
< wumpus>
it sounds like the way to go for e.g. embedded devices
< wumpus>
but certainly not if you run desktop applications like browsers :-)
< Luke-Jr>
dcousens: note that Linux likes to OOM-kill processes long before it runs out of RAM, if you don't have swap :P
< dcousens>
gmaxwell: I know. Eh, I've always kept that at the default, though, I'd be half tempted to turn it off if it meant I'd actually get std:error in my logs haha
< dcousens>
Luke-Jr: hasn't been my experience haha, I don't run swap on my most of my work machines and if I OOM, it just grinds to a complete halt usually
< wumpus>
I used to have no swap enabled when 8GB still seemed like a lot of memory, but I can confirm what Luke-Jr says, even compile with -j3 would invoke the killer penguin
< wumpus>
the problem was that any amount of swapping somehow instantly brought the entire system to a halt (even mouse cursor would no longer move) until it was done. Managed to improve this somewhat by setting high swappiness.
< wumpus>
(which makes it swap little used pages in advance instead of as last desperate measure)
< Luke-Jr>
nice thing about a desktop PC is I have like 4-6 hard drives I can RAID0 the swap across..
< wumpus>
good idea
< Luke-Jr>
(fwiw, that's builtin to Linux by using swapon -o pri=N where N is identical for everything)
< morcos>
I have some questions about changes I'd like to make to manual transaction prioritization and CreateNewBlock and block priority space.
< morcos>
I added a fee rate sort to the mempool. Using this and examining 600 calls to CreateNewBlock over the last 12 hours, the time to create a new block has been reduced from 2.337 seconds + 222 ms to validate to 11 ms + 345 ms to validate
< morcos>
I think the increased validation time is because the originaly version made the cache warm first
< morcos>
However, this does not create a priority space and does not apply manual transaction prioritization
< morcos>
Fixing the manual transaction prioritzation shouldn't be a problem as long as we're willing to store that data in the CTxMemPoolEntry. I know it was previously discussed not to do that because it might only apply to a few txs
< morcos>
However I don't think a sort based on the modified fee will work very well otherwise. So question 1, can I store the fee delta in the CTxMemPoolEntry.
< morcos>
Question 2: I assume I'm not allowed to get rid of block priority space?
< morcos>
I was thinking I could just basically repeat the priority sort of the old alogrithm first, but I believe we might lose some advantages of the speedup. However I think we could just skip that if blockprioritysize=0. The code will be a bit uglier that way, but sort of optimized for the case where blockprioritysize=0?
< morcos>
Any thoughts?
< morcos>
Question 3: BlueMatt: In your issue about tie-breaking you seem to suggest its random. But isn't the current tiebreaker (for the fee sort) priority? What would you like the tiebreaker to be?
< morcos>
BTW, since the current tie-breaker is priority, and its expensive to calculate priority, I'm not going to try to make my code generate the exact same blocks as the old code.
< morcos>
sipa: gmaxwell: wumpus: see above questions please
< sipa>
morcos: BlueMatt's problem occurs when there is an abundance of transactions with the exact same priority, i think
< sipa>
morcos: i too believe we should get rid of priority, and replace it with a custom function that alters size or fee of a transaction
< gavinandresen>
sipa: I agree, but a simple function with a miner-tweakable multiplier (maybe zero by default to mean "no priority adjustment") would be fine, I think.
< sipa>
gavinandresen: right, i was taking code-wise... the policy code can implement an overridable function, and we know that everything keeps.workimg whatever that function is
< sipa>
the default implementation can be a simple multiplier
< sipa>
or a constant that get added to both fee and size
< gavinandresen>
spiffy, I was worried somebody would go and implement a javascript interpreter so miners could tweak the function at runtime... :-)
< sipa>
ha
< wumpus>
should definitely use lua for that *ducks*
< sipa>
we should use bitcoin script with the op_x86 extension
< wumpus>
morcos: what is the main drawback of storing the fee delta in CTxMemPoolEntry? increasing the size of the structure?
< wumpus>
bitcoin script, hah, even better
< wumpus>
morcos: optimizing for blockprioritysize=0 sounds good to me, removing it is somewhat controversial - but as it seems to be where things are going anyway, and simplifies the algorithm a lot it makes sense to optimize for the case
< morcos>
ok thanks everyone. yes only downside is more bloat to the mempool, but its relatively small.
< morcos>
sipa: so perhaps i'll make a CTxMemPoolEntry function GetMiningPriority (ugh I hate overuse of the word priority) which for now = GetFee() + feeDelta
< morcos>
or should I store a miningPriority variable, and use an outside of CTxMempool function to calculate it and set it? I guess it could set it at creation and further calls to prioritizeTransaction would adjust it?
< jonasschnelli>
cfields: any idea why i get "clang: error: unknown argument: '-framework QtCore'" on my osx machine with qt5.5 [homebrew]?
< cfields>
jonasschnelli: mm, that's an odd error. is anything quoted funny?
< jonasschnelli>
cfields: i just switched back to Qt 5.4.1 .. there i don't have a such problem..
< jonasschnelli>
cfields: no additional details... just the clang error output during linking
< cfields>
jonasschnelli: can you paste the exact link-line? (make V=1)
< cfields>
if there's a problem with the framework, i'd expect "framework not found" or so
< jonasschnelli>
cfields: will do soon... currently switched back to qt5.4.1
< bsm1175321>
Can discuss c++11 here if you prefer. But then what's #bitcoin-dev for?
< wumpus>
for more abstract things, nitty gritty implementation details for bitcoin core belong here
< bsm1175321>
wumpus: indeed an explicit shared_ptr gets rid of the compiler error. Let's see if it runs...
< GitHub176>
[bitcoin] Diapolo opened pull request #6893: forward-declare univalue in rpcclient + remove include in rpcserver.cpp (master...univalue) https://github.com/bitcoin/bitcoin/pull/6893
< btcdrak>
gmaxwell: Not sure if we need to backport to 0.10, didnt have time yet anyway.
< gmaxwell>
ah okay I missed it.
< btcdrak>
gmaxwell: I also made a branch with BIP68+OP_CSV in one as some people asked for it. I'm debating if I should make a PR or just leave the two separate PRs as is.
< gmaxwell>
btcdrak: this patch does not appear to enforce (as standardness) the new behavior.
< morcos>
there are some unit tests in miner_tests.cpp which verify that CreateNewBlock won't mine non-final txs that are in the mempool
< morcos>
they were added by petertodd
< morcos>
i was under the impression that we don't want to allow non-final txs into the mempool
< morcos>
i suppose i should have checked that my code passed the unit tests earlier on, but the whole idea is to not recheck everything while building the block
< gmaxwell>
morcos: the finality test is against the mined block itself, while the mempool is running one block behind.
< morcos>
i think that was fixed so the mempool checks against +1, but that would have been a problem in the opposite direction
< gmaxwell>
So the mepool need a one block lookahead or there will be an extra delay in mining transactions. (which would presumably mean a competative advantage for miners that change that code)
< morcos>
the unit test specifically calls addUnchecked with a non-final tx, and then sees if CreateNewBlock would mine it
< morcos>
non-final for 1 block ahead (and a time based version as well)
< morcos>
what i'm saying is I'd like to be able ot rely on there being no non-final txs in the mempool, i think this was the idea sipa had in mind too?
< morcos>
the one thing I can think of is that its possible we need a function analogous to removeCoinbaseSpends for locktime based txs in the event of a reorg if thats not happening already
< gmaxwell>
The hight based can be precise, e.g. so it's not possible to add something that won't be final; and I think that would be good enough for what you're doing. But time based can't be precise; but MPL is fixing that. I'm fine with there beinging nothing that won't be final.
< gmaxwell>
I'm not sure if we're on the same page, so I'm saying that I think there should be currently non-final tx in the mempool which will be final from the perspective of createnewblock, but none that won't be final from the perspective of createnewblock.
< gmaxwell>
Before MTP this was hard to achieve without adding additional delays, but MTP will effectively make that additional delay universal.
< morcos>
I guess thats the same page. Mempool adds 1 to chainActive.height before checking finality, so i would consider it to be final from the perspective of both the mempool and CNB
< morcos>
and yes i agree that the current state is no non-final txs will end up in the mempool
< morcos>
what i'm saying is the unit tests require that even if they do, CNB will not put them in the template. (that breaks for me, and i think is unnecessary computation)
< gmaxwell>
okay, fair enough, I was defining current as chainActive.Tip().
< morcos>
also what i'm saying is that a reorg could probably break that assumption
< gmaxwell>
I think we can drop that test, but we need to make sure that reorgs are removing things correctly at the same time. (I don't know if we have a test for that).
< sdaftuar>
morcos: bluematt had a pr recently to fix that
< sdaftuar>
#6595
< sdaftuar>
we need to pick that up again...
< sdaftuar>
oh ha. i remember now, we were going to revamp the way reorgs are handled
< morcos>
ah, ok good. yes we need that.
< morcos>
and yeah thats what i was thinking, that you only want to run this after the reorg
< morcos>
but why is that difficult
< sdaftuar>
i'll dust off that code. after packages went in, bluematt's open pulls needed some rebasing
< sdaftuar>
see also #6656
< morcos>
you odn't have to redo the whole way reorgs are handled, you just have to call the removeForReorg after ActivateBestChainStep returns
< sdaftuar>
yeah i think that's what 6656 does
< sdaftuar>
er, did
< morcos>
perhaps you need to flag that the height or mediantimepast might have changed
< morcos>
down
< gmaxwell>
Perhaps that unit test could be changed to addunchecked but then trigger the logic that handles a reorg.
< morcos>
heh, i think thats in a different unit test
< morcos>
sdaftuar: so 6656 really supercedes 6595 then. jsut for some reason 6595 was left open
< morcos>
gmaxwell: this actually does lead me to a concern. i was leaving in the final TestBlockValidity test, but what should the code do in the event it fails?
< morcos>
I'm thinking it should switch into mining empty block mode, because otherwise there is no way to figure out what part of your block assembly broke
< morcos>
the down side of this, is if there is some error in mempool code that lets non valid txs in, all miners will just start switching to mining empty blocks
< morcos>
ugh that sounds terrible
< gmaxwell>
We assert now, no? .. yea, but I was just about to suggest it could start doing that. OTOH, that doesn't exactly encourage people to fix things, and empty blocks may mean that you don't failover to another daemon.
< morcos>
its much less likely to happen now, because we CheckInputs on every tx as we add it anyway, and if that fails we skip that txs
< gmaxwell>
morcos: well empty blocks are safe(er). Assuming that _prior_ chain is not invalid, they're no worse than not mining and likely better.
< morcos>
perhaps i should put that code back in
< morcos>
and just leave it off, unless you hit an invalid block, and then switch it on
< morcos>
because that way you'll just skip over the invalid txs
< morcos>
at least for that failure mode
< gmaxwell>
Running checkinputs again sounds expensive.
< morcos>
but then it won't be a very tested code path, unless we have a mode to test it
< morcos>
yeah well thats why the existing code is slow
< morcos>
its a nice property of the existing code, that if somebody injects an invalid tx into mempools, the mining code will just keep skipping it and assembling valid blocks from the rest of the txs
< gmaxwell>
In any case, thinking in terms of what the user of this code wants; they either want empty blocks (if they have no fail over) or they want the node to stop responding (if they have fail-over).
< gmaxwell>
morcos: right, it keeps the mempool behavior seperate from consensus.
< sipa>
morcos: i think the mempool should avoid but not outlaw non-final transactions; CNB should skip the nonfinal ones though
< gmaxwell>
what about checkinputs redundancy? at least doing another IsFinal check is cheap.
< sipa>
i think all mempool entries should be valid regarding 1) not spending nonexisting inputs 2) script validity
< sipa>
if we have that, i think CNB could just not validate tye result
< sipa>
but that seems controversial
< morcos>
sipa: so you'd like me to put IsFinal check back in?, you might be right, it might be cheap...
< morcos>
i suppose it would be nice to not have to worry about removing txs in the event of a reorg, you know they'll be valid again very soon
< morcos>
what do you think about BIP68 and sequence numbers though
< morcos>
thats not cheap to check b/c you have to look up the inputs
< gmaxwell>
sipa: really if we make CNB no longer latency critical (via mechenisms discussed before) then I think validate after remains a cheap protection.
< gmaxwell>
but I'd rather remove some of the internal double checking where it is expensive and really shouldn't be needed but continue to validate after.
< sipa>
morcos: i think enforcing nonfinalness in the mempool.is expensive, and the worst a reorg can cheaply do is put a bunch of things in that will be check anyway
< sipa>
morcos: fair enough, bip68 does make this significantly more expensive
< morcos>
sipa: i feel like i'm really overloading CTxMempoolEntry, but seems like it could store a block height and time at which the tx is final
< morcos>
that way the locktime and bip68 checks that you do at ATMP don't have to be repeated (ugh... i guess unless there is a reorg that affects inputs)
< sipa>
morcos: sgtm, if that can guarantee not needing to verify after creation
< morcos>
sipa: well then we're back to the reorg problem right? but maybe that's solvable more quickly by only having to do work on smaller subset of the txs. for instance if you know the lowest height and mediantPast time you disconnected down to, you only need to look at txs with CTxMemPoolEntry valid time and heights > that
< GitHub163>
[bitcoin] giacecco closed pull request #6885: Instructions on how to make the Homebrew OpenSSL headers visible... (master...master) https://github.com/bitcoin/bitcoin/pull/6885
< morcos>
sipa: BlueMatt: sdaftuar and i were just talking. the ccoinsviewcache growing out of control. the biggest problem is a reorg! a reorg will cause removeCoinbaseSpends to run, which will pull every single txin into the cache
< morcos>
i assume it'll flush right after that, but at that point it'll be too late if you OOM'ed
< sipa>
hmmm
< morcos>
i think storing this valid height/time will be very useful for coinbase spends, and moderately useful for checklocktime and csv
< morcos>
arguably we should actually store 3 numbers, the height/time its valid at, and the height which if you reorg below you need to recalculate validity
< morcos>
then on a reorg we just scan the whole mempool checking that last number, and recalculate validity
< morcos>
unfortunatley anti-fee sniping will cause any txs which have arrived in the last couple blocks to be triggered for recalc
< morcos>
and wihtout storing more info, you'd have to recalc for coinbase, checklocktime and bip68 sequence, bc you don't know which was invalidated
< morcos>
so its a bit of an expensive recalc
< BlueMatt>
morcos: no, there is no current tie breaker in CreateNewBlock
< morcos>
BlueMatt: sure why not, it uses TxPriorityCompare right? which sorts by fee then priority
< BlueMatt>
morcos: the way I read it, it first creates a heap according to priority, then resorts to create a heap using fee
< BlueMatt>
it never uses fee then priority
< morcos>
yes correct, but if you look at that compartor, in either case it uses the other one as the tie break
< morcos>
comparatator
< BlueMatt>
hmmmm, indeed, I hadnt seen that before....how is it that ordering seems randomish, then?
< BlueMatt>
lemme go look at mempool
< morcos>
yeah i was wondering that, you would think that would be fairly consistent across nodes
< morcos>
i wonder if in this recent attack there are many things with also the same priority... seems a bit unlikely..
< sipa>
that was my assumption
< sipa>
if all outputs are created in the same block, and the spends are the same size
< sipa>
that's what you would get
< BlueMatt>
morcos: hmmmm, now I'm very confused
< BlueMatt>
morcos: there are almost no txn in my mempool which have the same fee and prio
< morcos>
BlueMatt: I didn't look into the root issue, its just that you are not seeing the blocks you expect to see? It's not a result of malleated txs? Where some nodes have one version and others have another
< BlueMatt>
it could be related to malleated txn, but ive definitely seen it every time mempools get large, not necessarily when malleation is happening
< sipa>
might it just be different miners using different policies?
< BlueMatt>
afaiu there are effectively no different policies in use by miners today