< btcdrak>
Twitter reached out shortly after taking down the impersonator account down. They apparently have done their research and were well aware of the situation.
< maaku>
style question: when making something like an STL container, should I still adhere to the bitcoin style guide (e.g. CamelCase methods)?
< sipa>
at least for two examples of those (limitedmap and prevector), we follow STL naming conventions, but that's mostly because these are actually drop-in replacements
< Guest76632>
hi
< Guest76632>
How can I make a request to update the list of exchanges on bitcoin.org?
< pbase>
hi i am currently syncing the core wallet which is pruned. Can i copy paste the entire downloaded data for a second wallet on a different computer (local). basically what i want to do is to save time in syncing the data on the second computer
< sdaftuar>
;;diff
< gribble>
1.123863285132E12
< timothy>
pbase: yes
< pbase>
timothy, just delete the wallet.dat on the new computer?
< pbase>
second computer*
< promag>
ryanofsky: care to review? ty!
< CubicEarth>
jonasschnelli: You here?
< jnewbery>
sipa: what do you think of https://github.com/jnewbery/bitcoin/tree/pr11389.2 ? It's your original approach for #11389 rebased on a commit to pin P2SH to a block height. That seems more palatable than pretending P2SH is a VB deployment.
< sdaftuar>
jnewbery's branch looks correct to me (and simpler)
< jnewbery>
sipa: I believe so - I think the behaviour is equivalent to your change that uses version bits
< jnewbery>
obviously needs thorough review though
< sipa>
jnewbery: but not configurable
< sipa>
i needed to at least modify one unit test
< jnewbery>
sipa: ah yes, you're right
< jnewbery>
the failing test in miner_tests.cpp is testing that template creation fails when including a P2SH output. I think we can just remove it
< sipa>
jnewbery: fair
< sdaftuar>
is there a reason we should not set fAddnode=true for -connect peers?
< sdaftuar>
(asking because i am treating addnode peers specially for things like #11490, and it seems like -connect peers should get the same treatment)
< sipa>
sdaftuar: when using -connect, the normal connection-creation loop doesm't even run, so i guess that for most things it doesn't matter
< sipa>
but if it does for 11490, it should probably be changed
< gmaxwell>
sdaftuar: think it would be fine to addnode.
< sdaftuar>
ok i'll throw a commit in that sets fAddnode, and if someone comes up with a reason to use a new bool I can switch
< sdaftuar>
gmaxwell: thanks
< jnewbery>
sipa: it was a bit more complicated than I thought. The miner_test was testing that having an invalid P2SH tx in the mempool would cause template creation to fail. Currently, P2SH is always enabled in the unit tests (because it's activated using system time). Pinning the activation height of P2SH means that P2SH is not active so the test was failing.
< jnewbery>
Rearranging the ordering of the tests so that the P2SH test only happens after height 210,000 appears to be sufficient to fix it
< sipa>
cool!
< sipa>
have a branch?
< jnewbery>
same branch. Final commit fixes the test
< BlueMatt>
sdaftuar: oh, i thought i mentioned to you i had a pr that changed the definition of fAddnode - I'd recommend you just pull that commit into your pr
< sdaftuar>
i thought i remember you saying that we should treat connect peers the same as addnode peers, but didn't remember exactly what that entailed. cool, will do
< cfields>
make sure to take the next one that renames too, to avoid colliding when rebasing
< gmaxwell>
sdaftuar: re your excemption of addnoded peers, I think thats right for now but perhaps not what we want long term.. consider the case where you've configured 20 addnode peers... 8 are connected (per the limits). Two of the eight are forked nodes... you'd probably prefer to drop them and try to replace them with some of your additional configured options.
< gmaxwell>
so I think the longer term test should be "are my outbound connections of this type full, if not, don't disconnect anything from that type"
< sdaftuar>
gmaxwell: i thought addnode peers are in addition to our 8 outbound peers nowadays, did i misunderstand?
< gmaxwell>
yes, there are 8 additional addnode peers.
< sdaftuar>
oh i see
< gmaxwell>
(keeps people out of ignorance from addnoding the whole network to their detriment)
< gmaxwell>
and in the above the definition of full also means "more to try are available"
< gmaxwell>
if you have 8 or fewer addnodes configured you should never disconnect one.
< gmaxwell>
in any case, I don't think we need to be smarter now.
< sdaftuar>
i'm struggling a little to come up with an outbound peer evicter that kicks in when we haven't advanced our tip in a long time
< sdaftuar>
it seems like that logic wants to live in net.cpp
< sipa>
sdaftuar: net shouldn't know about a 'tip'
< sdaftuar>
yeah i know... i think net_processing would fill in information, but net is the best place to look at your set of peers, i think?
< sdaftuar>
like we do for inbound peer eviction
< sipa>
right, it's a bit fuzzy for now
< cfields>
sdaftuar: that set of peers is moving out, but it's not clear how we're going to accomplish that
< sipa>
sdaftuar: i think eventually all the information that we'd use to determine who to evict will live in net_processing, apart perhaps from connection uptime (which it could easily query net for)
< cfields>
sdaftuar: ideally, PeerLogicValidation (needs rename) holds a vector of CNodeState. But there are lots of locking issues to get through before we get there :(
< cfields>
(the idea being that if you want to do something for each peer, you look there, rather than the CNodes in net)
< cfields>
yes, what sipa said too
< sdaftuar>
the thing i'm trying to do now is look at my set of outbound peers, pick the worst of them under some metric, and disconnect it -- but only if i'm full and am using up all my outbound slots
< sdaftuar>
it doesn't seem easy to determine the last thing from net_processing
< sdaftuar>
but the metric uses data that only net_processing knows
< cfields>
sdaftuar: you can use a ForEachNode. That was kinda meant to be a stop-gap for stuff like this until it's all moved out.
< BlueMatt>
sdaftuar: I think in that case you should prefer to add new state to CNodeState/net_processing.cpp static
< BlueMatt>
(net_processing.cpp static will probably all eventually move into PeerLogicValidation as it gets renamed)
< sdaftuar>
i think i need to expose ConnMan's nMaxOutbound in order to do this outside of net -- does that seem ok?
< BlueMatt>
why is that not a #define anymore :(
< cfields>
so that we can test changes exactly like this one :)
< cfields>
sdaftuar: how about testing each node and marking exactly 1 at a time as preferred for eviction? Then the connection loop can check for !outgoing_full || outgoing_full-1 && have_evictable ?
< cfields>
er, that logic's all busted but you get the point :)
< sdaftuar>
ah, i think i like that
< sdaftuar>
thanks, i'll give that a try!
< cfields>
np
< bitcoin-git>
[bitcoin] jnewbery opened pull request #11495: [trivial] Make namespace explicit for is_regular_file (master...explicit_is_regular_file) https://github.com/bitcoin/bitcoin/pull/11495