< bitcoin-git>
bitcoin/master 2e44893 Jonas Schnelli: Move -salvagewallet, -zap(wtx) to where they belong
< bitcoin-git>
bitcoin/master 40022fe Wladimir J. van der Laan: Merge #9142: Move -salvagewallet, -zap(wtx) to where they belong...
< bitcoin-git>
[bitcoin] laanwj closed pull request #9142: Move -salvagewallet, -zap(wtx) to where they belong (master...2016/11/wallet_init) https://github.com/bitcoin/bitcoin/pull/9142
< wumpus>
can people please notify me if there is a pull which is clearly ready for merging? (e.g. 9142 had three utacks and was ready for merge for ~7 days already)
< wumpus>
there are too many open for me to keep track of so I really need help in that
< wumpus>
I had seen it before and wondered wtf, but seemed too invasive to change at the time
< cfields>
wumpus: well the zero-write goes out of its way to be a pain-in-the-ass, and therefor slower than it should be. Otherwise it'd just be optimized out
< wumpus>
now that things have beeen disentangled a bit it is easier
< cfields>
so i think much of the hit just comes from having any non-trivial dtor, regardless of what it is
< cfields>
but again, that's coming from profiling data in the non-optimized case. Much of that probably goes away at -O2 or -O3
< wumpus>
yes could be
< cfields>
so yea, the locks probably account for most of it at that point
< wumpus>
right, profiling results at O0 are useless :)
< cfields>
wumpus: i can't think of where cleansing is really necessary other than private keys and passphrases?
< cfields>
though i'd like to be sufficiently paranoid if i'm going to be the one PRing the removals :)
< wumpus>
I think so, I'm not entirely sure
< wumpus>
if that is the case we wouldn't even need the zero_after_free allocator, just the lockedpool one used for key data
< wumpus>
but in any case yes such a PR would have to evalutate all uses of zero after free individually
< cfields>
that makes sense. Hard to make the case for something sensitive enough to zero but not memlock
< fanquake>
Does anyone have some inputs to share for testing #9172
< fanquake>
cfields talking about 9172? Did you install afl via brew or download and install?
< fanquake>
wumpus thanks
< cfields>
fanquake: not tested yet, just a quick review of the build changes
< fanquake>
Ok. I'll try using brew'd afl, seems to be up to date (2.35b)
< wumpus>
cfields: "Note that each message should be padded in the front by at least CSerializedNetMsg::header_pad_size bytes. This allows for a header to be inserted without requring a copy." awesome, we're getting our own skb's :)
< cfields>
wumpus: hah! that's exactly what it's like. Wish I would've known to google "skb" earlier, would've saved me some head-scratching. Good to know it's a reasonable approach, though :)
< wumpus>
instagibbs: eh in #9073 you changed the bitcoin-cli message but it's confusing now: error: couldn't connect to server (make sure server is running and you are connecting to the correct RPC port: -1 unknown)
< gribble>
https://github.com/bitcoin/bitcoin/issues/9073 | Trivial: Add common failure cases for rpc server connection failure by instagibbs · Pull Request #9073 · bitcoin/bitcoin · GitHub
< wumpus>
instagibbs: that's as if the *port* is unknown. The error code and message should be on the first line, not the second :)
< wumpus>
cfields: yes it's a great way of handling packet processing
< bitcoin-git>
bitcoin/master f004e67 Greg Walker: Minor change to comment above new NODE_WITNESS service flag to keep it consitent with existing comment structure. Helps with readability.
< bitcoin-git>
bitcoin/master 5e8631b Wladimir J. van der Laan: Merge #9205: Minor change to comment for consistency....
< jonasschnelli>
luke-jr: If I call invalidateblock(d) on a-b-c-d-e, the tip will chain will result in a-b-c
< luke-jr>
jonasschnelli: it shouldn't be, if there is an alternate chain a-b-c-x
< luke-jr>
to test, send a-b-c-x then reorg to -d-e then invalidate d
< BlueMatt>
wumpus: might want to cancel meeting on thursday in light of american thanksgiving
< wumpus>
BlueMatt: good point
< BlueMatt>
wumpus: also, before you get too excited about #9128, I think sipa, cfields and I need to get togehter (or at a meeting) and figure out what we want to do about where-messages-get-hashed
< wumpus>
BlueMatt: cfields could we possibly factor out the controversial part into a seperate pull?
< BlueMatt>
wumpus: I think we just need to find a few minutes to discuss when we're all here...it should be easy to come to agreement (hence why i came to the conclusion that this week's meeting wont work :( )
< wumpus>
most of the changes are no-brainers w/ regard to desirability
< BlueMatt>
yea
< wumpus>
but sure we can hold off on that one until you discussed
< BlueMatt>
wumpus: I keep remembering after cfields goes to sleep :(
< gmaxwell>
wumpus: ACK re-pings. I have seen some sit ready for a while and didn't want to nag.
< jonasschnelli>
luke-jr: I think that would be a general improvement in InvalidateBlock ...
< jonasschnelli>
luke-jr: heh, InvalidateBlock(getbestblockhash()) on a fresh regtest setup (no block mined) results in a funny state
< luke-jr>
does it really not work? :O
< jonasschnelli>
It disconnects the genesis block but missed the undo-data
< jonasschnelli>
generate 1 will fail afterwards
< luke-jr>
no, I mean a-b-c-x
< * jonasschnelli>
testing
< luke-jr>
I mean, it's rare you'd want to invalidate the tip without a competing chain to reorg to
< jonasschnelli>
I think InvalidateBlock is only used during RegTests/RPC tests... are there real world mainnet usecases?
< BlueMatt>
so I could PR relay-compact-block-before-full-validation (https://github.com/TheBlueMatt/bitcoin/commits/2016-11-compact-fast-relay) but now I'm kinda thinking I dont want to do that prior to there being multithreaded ProcessMessages with some smarts to respond to getblocktxn without a lock for the latest block...thoughts?
< BlueMatt>
(maybe gmaxwell)
< luke-jr>
it was added so a fiasco like 0.8.0 could be manually dealt with
< jonasschnelli>
luke-jr: generating a-b-c-d-e, invalidating e, mining -d-x-y, reconsidering e, invalidate x results in a-b-c-d
< jonasschnelli>
So reorg with InvalidateBlock seems to work
< jonasschnelli>
Maybe the pprev call is not ideal
< luke-jr>
that sequence doesn't make sense. you mined d twice?
< luke-jr>
if a-b-c-d-e is valid, tip should never be D
< jonasschnelli>
I invalidated e
< luke-jr>
but then reconsidered it
< luke-jr>
which should make it valid
< jonasschnelli>
wait... let me write it again
< jonasschnelli>
generating a-b-c-d-e, invalidating e, mining (-d)-x-y, reconsidering e, invalidate x results in a-b-c-d-e
< luke-jr>
ok, good; but does this call NotifyTip with D or E?
< jonasschnelli>
I'm checking now the signal... maybe its wrong.
< luke-jr>
it should only call NotifyTip with E I think, but certainly both and E last
< luke-jr>
err, certainly E last*
< luke-jr>
(whether it calls it with D before E is a debatable question)
< gmaxwell>
BlueMatt: I think it's fine to send it first without fancy getblocktxn handling. Most blocks relay without a getblocktxn.
< gmaxwell>
BlueMatt: on the balance for both bandwidth usage and latency handling using the orphan pool and having an eviction pool be more important that faster getblocktxn handling.
< gmaxwell>
BlueMatt: I think sipa and I convinced cfields that what you were doing was reasonable.
< gmaxwell>
luke-jr: jonasschnelli: I'd be really surprised if notify behavior was sensible around invalidate block, it's a hidden and undocumented command intenteded for development and for fringe emergencies. (Though I'd certantly support making it more sensible-- AFAIK it really hasn't been tested to the level that I'd expect for a first tier user feature).
< gmaxwell>
It also is massively slow at undoing blocks, enough to degrade its utility.
< luke-jr>
gmaxwell: jonasschnelli just "fixed" it, but I think the fix is wrong
< jonasschnelli>
luke-jr: I don't fixed it,... I merged a fix. :)
< jonasschnelli>
But I guess luke-jr is right... it could be wrong.
< jonasschnelli>
But my test just showed me, the signal gets called with the reconnected blocks.. so the UI state is correct... but looking deeper into it now
< gmaxwell>
One challenge is that invalidateblock can cause a best block tip with lower work than the best signaled previously. There is no other way for that to happen in the codebase, so in that sense it breaks an interface invarient.
< luke-jr>
well, at least that case is GIGO
< jonasschnelli>
luke-jr: I think the change is correct
< gmaxwell>
true. :)
< jonasschnelli>
pprev = chainActive.Tip()
< jonasschnelli>
And in the RPC call InvalidateBlock, we have a call ActivateBestChain(state, Params(), NULL);
< jonasschnelli>
Which calls the signal again with the new tip
< jonasschnelli>
so.. all good IMO
< luke-jr>
ah, so the internal InvalidateBlock doesn't complete the reorg(s), and the RPC does that in a subsequent step?
< jonasschnelli>
Yes
< luke-jr>
I guess that makes sense. Maybe better to change it, but outside the scope of this.
< sdaftuar>
gmaxwell: ^ This should fix the invalidateblock performance issues
< brhood9>
is anyone there? I need help please and I'm willing to pay.
< Victorsueca>
brhood9: what with?
< bitcoin-git>
[bitcoin] MarcoFalke closed pull request #9139: Change sync_blocks to pick smarter maxheight (on top of #9196) (master...sync-height) https://github.com/bitcoin/bitcoin/pull/9139
< bitcoin-git>
bitcoin/master 1126c85 Russell Yanofsky: [qa] Change sync_blocks to pick smarter maxheight...
< bitcoin-git>
bitcoin/master 0de7fd3 MarcoFalke: Merge #9139: Change sync_blocks to pick smarter maxheight (on top of #9196)...
< bitcoin-git>
[bitcoin] MarcoFalke closed pull request #9139: Change sync_blocks to pick smarter maxheight (on top of #9196) (master...sync-height) https://github.com/bitcoin/bitcoin/pull/9139
< brhood9_>
This is about unconfirmed transactions that need to be fixed.
< brhood9_>
Is there any way that you all could help me figure out what is going on with my bitcoin transaction? I have asked and looked and read everywhere. The transaction that you need to look at is this one.
< Chris_Stewart_5>
There isn't an easy way to replace a tx in the mempool if the user had all sequence numbers set to max, correct?
< Chris_Stewart_5>
and the parent isn't RBF either
< gmaxwell>
in practice you just create a replacement and keep broadcasting it. miners get restarted enough that it's pretty likely to get replaced eventually.
< gmaxwell>
(assuming the prior was low fee)
< Chris_Stewart_5>
Yes, it happened before this latest 'influx' or txs. Does Core's algorithm rejects txs from the mempool if it already has a tx that spends that particular output? I'm guessing it doesn't match on txids since those will be unique assuming
< Chris_Stewart_5>
some sort of malleability or reordering of inputs
< BlueMatt>
jonasschnelli: if you're gonna ack #9183, please also ack its dependencies, #8930
< cfields>
BlueMatt: so, before we end up in tangential discussions, i think the key thing to decide on is where encryption state should be held
< BlueMatt>
cfields: fix the TODO :p
< BlueMatt>
especially, send the message without hashing the message repeatedly
< cfields>
BlueMatt: and if some nodes are using encryption?
< gmaxwell>
BlueMatt: you're not thinking you can reuse the authentication value across multiple peers when encryption is in use, are you?
< BlueMatt>
gmaxwell: no, but for current-style peers you can :p
< sipa>
well i think cfields's design was based on the goal of being able to construct a message once, and then send it to multiple peers
< cfields>
BlueMatt: fwiw, i had a discussion with sipa/gmaxwell about my grand plans for dedup, which, they made me realize doesn't make much sense
< BlueMatt>
cfields: in any case, it seems massively layer-violating to have any knowledge of the structure of bytes-on-the-wire in CConnman
< sipa>
BlueMatt: i don't see that
< BlueMatt>
sipa: my impression is that the goal of CConnman is to be a network management library
< sipa>
CConnman shouldn't know about how a block or inv message is structured
< BlueMatt>
not a bitcoin-p2p-management library
< sipa>
but it certainly should be able to construct and dissect network messages?
< BlueMatt>
why?
< sipa>
where else does that logic belong?
< BlueMatt>
what if we want to change the header for encryption
< cfields>
BlueMatt: at minimum, it has to have knowledge of when a message is complete
< cfields>
well it doesn't have to, but for efficiency, it should
< BlueMatt>
the logic belongs int he message-serialization/-deserialization logic
< gmaxwell>
I really don't think it's worth the savings to conserve message hashing for the non-encrypted case, lets get everyone to encryption. :)
< BlueMatt>
cfields: yes, the MessageDeserializer class thinggy we have now works great :)
< cfields>
BlueMatt: which?
< gmaxwell>
Serialize once is fine...
< cfields>
BlueMatt: wait... let's stop there. I was trying to avoid getting this far without answering the first question...
< BlueMatt>
cfields: CNetMessage
< cfields>
<cfields> BlueMatt: so, before we end up in tangential discussions, i think the key thing to decide on is where encryption state should be held
< BlueMatt>
cfields: right beside current protocol versioning info in CNode?
< sipa>
i think we conceptually have 3 layers 1) network bytes 2) p2p messages 3) contents of p2p messages
< cfields>
BlueMatt: that stuff is now ready to be extracted, just waiting on CNodeState lock cleanups
< BlueMatt>
cfields: CNetMessage is an object which is going to have to be peer-specific
< sipa>
i think BlueMatt is trying to separate 1 and 2
< sipa>
cfields is separating 2 and 3
< BlueMatt>
sipa: yes
< sipa>
i'm of the opinion that separating 2 and 3 is the more interesting problem now
< BlueMatt>
cfields: yes, I dont think it should be in CNode, if you want CNode to be a "dumb thing that CConnman handles"
< BlueMatt>
but it should be in the same place
< BlueMatt>
sipa: p2p messages and contents of p2p message/processing thereof seem already relatively separated, no?
< cfields>
BlueMatt: imo, CNode should be just the stuff related to connection state. And current iv's fall into that category.
< BlueMatt>
cfields: a corollary to your question is how does CNetMessage become peer-specific?
< sipa>
so the question is whether CConnman should do the serialization of the contents of messages
< BlueMatt>
cfields: "ivs"?
< sipa>
if CNetMessage is peer-specific, it perhaps should be just hidden behind CConnman?
< sipa>
like it currently is
< sipa>
that makes reuse of messages across peers impossible, but maybe we're not interested if it can't be done at the same time as hashing/encrypting
< gmaxwell>
for example, some peers might use compacted transactions some might not. But is that the correct layer for it? I don't think we'd propose implementing BIP152 block compaction inside CConnman.
< cfields>
sipa: to be specific, message reuse is possible, it's only the header attachment that can't be shared
< BlueMatt>
sipa: the way i see it: processing of messages and the peer handling is already well-separated, or at least somewhat separated...we currently have bitcoin-specific and general-networking code intermixed in a bunch of places, and that also needs to be separated
< sipa>
cfields: i mean, the whole point of 9128 is the introduction of a serializer abstraction... that seems unnecessary if we don't plan on ever reusing network messages
< sipa>
cfields: while if it remains inside CConnman, it could do hashing/encryption on the fly during serialization
< cfields>
sipa: it's necessary as a layer abstraction... it's only a shortcut for callers
< BlueMatt>
sipa: the serializer-abstraction in 9128 is definitely useable even if we encrypt network messages, etc
< BlueMatt>
sipa: see my branch for early-sending-compact-blocks
< sipa>
cfields: i guess i don't see why
< cfields>
BlueMatt: fwiw, my first version used a shared_ptr for dedup, and cached the hash, so that the same message could be pushed to a bunch of peers without re-serializing or re-hashing. If you think that's worth putting back, i can.
< cfields>
heh, i'm not sure which conversation to be having here :)
< BlueMatt>
cfields: why shared_ptrs? that seems strange to me, but, then, I havent seen that version
< BlueMatt>
cfields: heh, we have fractured....
< sipa>
cfields: oops, i forgot the distinction between CSerializedNetMsg and CNetMsgMaker, ignore me
< cfields>
sipa: right. CSerializedNetMsg is intended to be a dumb payload. CConnman wraps it as necessary, then sends it with no knowledge of the contents. That's the layer separation change.
< sipa>
but i don't think we answered the question whether CNetMsgMaker should be node-specific or not
< sipa>
if we want it to do encryption on the fly, it should be
< sipa>
but that reintroduces the dependency on cconnman
< cfields>
sipa: it's only send-version specific, since that's all the serializers care about
< BlueMatt>
sipa: i mean thats the whole point of the discussion - is the thing we pass into CConnman a blind bytes array to send on the wire, or a serialized message, which needs its header attached, etc
< BlueMatt>
cfields: note that the way it currently is it mixes things...CNetMsgMaker adds most of the header, CConnman adds the hash
< sipa>
my impression is that 9128 intends CSerializedNetMsg to be the unencrypted thing, which is reusable across connections
< BlueMatt>
which seems like a layer-split
< cfields>
so we're back to the original question again of who holds the current encryption state :)
< BlueMatt>
cfields: first question - where are we putting current protocol version?
< cfields>
sipa: yes
< BlueMatt>
cfields: it was my impression you were planning on pulling that out of CNode and putting it in the State() stuff?
< sipa>
cfields: but it somehow still has an optimization of having padding space preallocated
< sipa>
cfields: which does not technically belong in that layer
< cfields>
BlueMatt: yes
< sipa>
cfields: hmm, but the State should be processing-module specific
< BlueMatt>
cfields: I believe encyption state should go right there...encryption state is something negotiated on connect, just like nVersion, and should be in the same layer
< BlueMatt>
sipa: it is
< sipa>
BlueMatt: disagree there - i don't think every network message handler should reimplement encryption
< cfields>
sipa: agreed. That's intended to be a "max padding necessary". I added it to make BlueMatt happy with zero-copy, at the cost of having to stick it at the wrong layer
< sipa>
it's a meta layer
< sipa>
there can be one module that deals with encryption, and once it's done negotiating, tells connman that from that point on encrypt all messages, including those from other modules
< BlueMatt>
sipa: " every network message handler should reimplement encryption" huh?
< sipa>
BlueMatt: my view is that there will be separate message handling modules, each with their own lock and their own state
< sipa>
BlueMatt: one from ping/pong, one for block processing, one for tx relay, ...
< cfields>
BlueMatt: i have a proposal for bip151 that would make it so that encryption isn't reliant on the messaging processing layer, but i haven't written it up yet
< sipa>
BlueMatt: and they install handlers for specific messages
< BlueMatt>
sipa: ok, and the place they hook in the handlers does the encryption, but certainly not CConnman
< sipa>
BlueMatt: but all of their sent messages should get encrypted, not only those from the encryption module
< sipa>
BlueMatt: i think you're envisioning a global processing layer in between the specific message processing and CConnman?
< BlueMatt>
sipa: what if we want to use CConnman for some other protocol - does it now have to have the same encryption protocol?
< BlueMatt>
sipa: CConnman should be a networking library, not a bitcoin networking library
< sipa>
BlueMatt: heh, i thought that the networkling library would be libevent or some lower level wrapper
< BlueMatt>
sipa: I mean right now its that way - the net message maker thing is a library that the processing modules all use
< sipa>
CConnman is exactly the bitcoin specific thing
< sipa>
cfields: opinion?
< cfields>
BlueMatt: that was my original intent. But it became clear as the months went on that tighter integration is needed...
< cfields>
BlueMatt: if we could get this stuff merged, we could move on to ripping out the lower-level stuff :)
< cfields>
after this comes the libevent+async changes, which is maybe closer to what you want to see changed?
< sipa>
BlueMatt: i would think that the separation you're talking about is maybe a later step... if we first want a net protocol, it just gets integrated into CConnman
< BlueMatt>
cfields: well I dont think you mean that tighter-integration is needed between bitcoin logic and libevent/async
< BlueMatt>
but clearly there is some integration needed where the net_processing stuff is aware of how to serialize a message
< BlueMatt>
or at least has an api like CNetMessageMaker
< cfields>
BlueMatt: yes, that's another possibility. One that sipa suggested a while back, i think
< BlueMatt>
cfields: what is?
< cfields>
callers could pass in abstract classes to handle the message wrapping (header)
< BlueMatt>
cfields: ultimately I think there /are/ clearly three layers here, as sipa said, just a quesition of how we split them up....I think for now its much easier to dump things in net_processing and make CConnman just a "dumb pipe" or at least "semi-dumb-pipe"
< BlueMatt>
because there are lots of changes that need to happen in just the network-library layer
< cfields>
BlueMatt: for context, I first wrote a completely low-level, no-bitcoin network layer. Plugging that into bitcoin proved to require a bunch of bitcoin-specific hacks. In the end, I didn't think it was an improvement over what we had.
< BlueMatt>
cfields: bitcoin-specific hacks like what?
< sipa>
BlueMatt: in your hypothetical new network protocol, are there still network messages of the form (12-byte-command, data) that are dealt with in the same way in main etc?
< kanzure>
was the networking library you were working on thrown out, cfields?
< BlueMatt>
sipa: hmm? I dunno, do we need to stick to the same header format in 151?
< BlueMatt>
sipa: I dont think we should be restricted to that?
< cfields>
kanzure: not thrown out, but i kinda decided to converge with our current stuff instead
< BlueMatt>
(plus I strongly thing having additional networking libraries would be welcome)
< sipa>
i see bip151 simply as a change to the pipe, not to the messages in it
< cfields>
BlueMatt: there's a strong reason to keep the header size, imo
< sipa>
and i think CConnman is what is common between all network protocols that are just pipes for delivering messages of that form
< BlueMatt>
sipa: indeed, but are message headers a part of the pipe, or the messages?
< cfields>
though i'm aware there are plenty of drawbacks, and i wouldn't be heartbroken if it were abandoned
< sipa>
BlueMatt: messages
< BlueMatt>
sipa: well putting them in CConnman is pipe....
< sipa>
BlueMatt: or you'd need to change every message handler too when the pipe changes
< BlueMatt>
(note that clearly the headers wont all be the same - bip 151 will at least change the definition of the hash field)
< cfields>
BlueMatt: i suppose we're looking at this from different sides. I originally rewrote the net layer and tried to plug it into bitcoin. I didn't like the result. I started instead trying to rip bitcoin out of our net layer, and that's where we are now. The next step would be to split the resulting CConnman into high and low levels. That's next.
< sipa>
BlueMatt: i see messages as (12-byte-command, payload) things... that excludes the checksum
< sipa>
bip151 changes the pipe, but the things being sent over it are still the same messages
< cfields>
sipa: note that if it remains with start bytes (different ones) up front, and padded somehow to 24 bytes, the net layer can handle decryption without having to count on the messaging layer for initialization
< BlueMatt>
cfields: ok, well if CConnman is gonna split, and the hashing/encryption/etc go in the high level, thats ok, but then CNetMsgMaker has to turn into what sipa said and have no knowledge of message-header
< BlueMatt>
(which I think is an easy change from 9128 as it stands now?)
< cfields>
BlueMatt: no knowledge as-in it's done in CConnman?
< BlueMatt>
cfields: its done in CConnman-high
< BlueMatt>
:p
< sipa>
i think it's fine for CSerializedMessage to have some padding space before/after for undefined purposes, which CConnman(-high) just happens to conveniently use
< BlueMatt>
sipa: I see no advantage to doing so?
< cfields>
right, that was only added for the zero-copy case
< sipa>
BlueMatt: not needing to copy it into a single buffer afterwards
< BlueMatt>
cfields: I dont think you need that? CConnman-low takes a list of vectors...just give it two instead of one
< BlueMatt>
its still zero-copy
< sipa>
hmm
< sipa>
i guess
< cfields>
alternatively we could just send twice, that was an original TODO to investigate
< cfields>
BlueMatt: is that what you're suggesting?
< BlueMatt>
cfields: "send twice"?
< cfields>
2 send() calls. or a sendmsg.
< BlueMatt>
yes
< BlueMatt>
thats ok
< BlueMatt>
(and not performance-busting)
< cfields>
(i'm pretty sure libevent uses scatter/gather anyway, so that should be freeish)
< BlueMatt>
even two syscalls wont be performance-bustingn
< BlueMatt>
ng
< BlueMatt>
well, do we still disable nagle?
< BlueMatt>
will have to look into that
< cfields>
yes
< BlueMatt>
have to use the MSG_MORE thing for the header, then
< sipa>
use writev(2) :)
< cfields>
BlueMatt: i was more concerned about possible awkwardness if you managed to push out exactly only the header
< BlueMatt>
before what? the connection dies? so what?
< sipa>
i do like this idea... just remove the message padding optimization out of CSerializedMessage, and use separate writes for header/...
< cfields>
ok, i figured someone would complain about that :)
< cfields>
easy, then
< cfields>
i'm not going to bother with writev/sendmsg though, since libevent will handle that internally
< BlueMatt>
thats fine
< BlueMatt>
if you switch to libevent and I run an strace and you're wrong, I'll be upset, though :p
< cfields>
heh
< cfields>
BlueMatt: so to be clear, the next steps are to begin making things async-friendly. I have a branch which adds OnConnect(), OnDisconnect(), etc
< BlueMatt>
cfields: cool, sounds good :)
< cfields>
once we're there, we can begin the separation, and have a virtual class (high level) that responds to those things
< BlueMatt>
sounds good
< sipa>
cfields: yay
< sipa>
concept ack
< cfields>
in the end, i think it will end up looking much like my original lib. We'll just have gone from core->lib rather than the other way around.
< cfields>
anyway, after this PR, I think we can segregate the files, and no one will have to concern themselves with the churn in the net code anymore :)
< BlueMatt>
cfields: yay, then I can split main :)
< cfields>
BlueMatt: heh, if we've done this right, we can do them in parallel without stomping on eachother :)
< BlueMatt>
cfields: nailed the timing too...you'll get your last change into main a week before it splits :p
< cfields>
hehe
< cfields>
BlueMatt: so to clarify, drop the pad hack, send() twice, and you're happy with the rest?
< BlueMatt>
cfields: yea, I'm ok with it as a next-step :)
< cfields>
BlueMatt: ok, thanks.
< cfields>
and thanks sipa as well
< sipa>
great
< cfields>
BlueMatt: i'm not familiar enough, so maybe you would know... any risk of the SyncTransaction having an effect on the downstream listeners? zmq, in particular
< cfields>
*SyncTransaction change
< cfields>
oh nevermind, i guess they don't actually see any difference
< BlueMatt>
cfields: yea, point is now the orphan map is limited to p2p logic now :)
< cfields>
BlueMatt: ready for the big move after #8930? Or am I missing one?
< dhill>
addrmap.cpp, in CAddrInfo::GetChance(), nSinceLastSeen is unused...
< fubu>
ssij
< fubu>
xD
< rusty>
Anyone thought about removing the dust limit? In a feemarket world, it seems a little useless, and for things like lightning it complicates things (we have to always keep everything above the dust limit otherwise risk holding a non-standard tx).
< gmaxwell>
If we consistently were operating in a functioning market I'd agree strongly.
< gmaxwell>
Unfortunately there are many blocks that aren't and so they'd mine huge wads of dust advertisment payments sitting around. I dunno if segwit will make this better or worse. (increases utxo costs, but also increases capacity.)
< gmaxwell>
rusty: so "someday hopefully soon"?
< gmaxwell>
already one large miner bypasses it, so perhaps we should give up.
< gmaxwell>
you can see there are many periods of effectively no backpressure.
< rusty>
gmaxwell: makes sense. In practice, lightning has a "don't bother making any outputs below this" for economic reasons, but version 0 might have skipped that if not for the standardness issue.
< rusty>
gmaxwell: it would be nice to have these nicely generated and running constantly somewhere, BTW. Did you have to hack bitcoind to get the data?
< fubu>
u
< gmaxwell>
rusty: no, set the target blocksize to 1mb... run
< gmaxwell>
I have it running on my desktop but I keep disrupting it to tinker with things.. though the disruptions are less disruptive now that we have mempool saving in master.
< gmaxwell>
the graph is just a little bit of postprocessing that data to get the before/after numbers for every block change.
< gmaxwell>
sipa was working on some patches that would give a "CDF" like report for the mempool. (size vs fees) multiple blocks out.
< gmaxwell>
one nice thing about this data is you can see all the miners that aren't doing CPFP mining yet... when they produce blocks that pay a lot less fee than my node would produce.