< raedah>
which file creates the info displayed in 'Show transaction details' ? Looking at src/qt/transactionrecord.cpp and src/qt/transactiontablemodel.cpp
< raedah>
ah, looks like its in transactiondesc.cpp
< GitHub190>
[bitcoin] pstratem opened pull request #7094: Assert now > 0 in GetTime GetTimeMillis GetTimeMicros (master...2015-11-24-assert-time) https://github.com/bitcoin/bitcoin/pull/7094
< gmaxwell>
Guess my PR answered the question of if we still had tests that were expecting the mempool RPC to behave in certian ways. :)
< phantomcircuit>
gmaxwell, it also failed on INT64_MAX for some reason
< phantomcircuit>
see my note on the outdated diff
< gmaxwell>
phantomcircuit: yea, I had a host that it did that on too. Which is weird because stdint.h is included in that file; but whatever; you were right: I should have done it consistently.
< phantomcircuit>
oh you fixed that already
< phantomcircuit>
i dont have any strong tie to either method but we've been using std::numeric_limits everywhere else (apparently for a reason, which i cant discern either)
< gmaxwell>
phantomcircuit: now the issue is that the blocktester (foolishly) depends on this.
< cfields_>
anyone happen to know why dns seeds register a second dns query result as their source in addrman, rather than some dummy value?
< gmaxwell>
er. it's supposted to be like.. the dns seed identifer as a source.
< cfields_>
gmaxwell: right. that identifier requires another resolve, which i suppose could fail and result in a source of "0.0.0.0".
< cfields_>
(or am i reading it wrong?)
< gmaxwell>
yea, it's brain damaged that it's resolving though, since thats going to return some random stuff.
< gmaxwell>
We should be filling it in with a dummy value and a distinct dummy value per dnsseed.
< gmaxwell>
I wonder if that resolution is a DNS leak when running on TOR?
< cfields_>
i don't believe so, it should be stopped later down the path
< cfields_>
er yikes.. maybe not
< cfields_>
gmaxwell: ok yea, that's ok. oneshots are used if a proxy is enabled, so that path isn't taken
< phantomcircuit>
gmaxwell, ./qa/pull-tester/rpc-tests.py passes all tests on 7093
< gmaxwell>
phantomcircuit: yea, it's matt's blocktester thats failing. It's monitoring node status via p2p mempool calls.
< phantomcircuit>
oh
< phantomcircuit>
BlueMatt, ^
< BlueMatt>
gmaxwell: why are you not responding to queries if the node just asked for mempool now?
< BlueMatt>
gmaxwell: that seems like an annoying hack :/
< gmaxwell>
BlueMatt: It already answered.
< gmaxwell>
it's not resending the same stuff it already sent.
< gmaxwell>
BlueMatt: you've been whined at before about the pull tester using mempool... mempool behavior isn't consensus normative. :)
< BlueMatt>
gmaxwell: yea, yea...
< BlueMatt>
gmaxwell: the goal of it has changed slightly over time
< gmaxwell>
in any case, I could add a hidden option to turn off this behavior, or bypass it for whitelisted peers or...
< gmaxwell>
but I do think we need to limit how mempool P2P command works (Jeff suggested removing it entirely; but it's widely used by SPV wallets)
< gmaxwell>
and I think my patch mostly solves the privacy and dos attacks without breaking SPV wallets (well, apparently mycelium redoes filterloads and mempools again, so I need some additional handling there)
< BlueMatt>
allow X-per-Y
< BlueMatt>
then everything works :)
< BlueMatt>
i think the tester only does like 3 or 4 total anyway
< gmaxwell>
well I'm breaking the tester because I don't return duplicate results.
< gmaxwell>
The delay means that a spv wallet will need to mempool again 30 seconds after connecting in order to get _everything_, and it would be nice to only send it INVs for the things it missed.
< BlueMatt>
yea, just multiply your quanitization interval by X and allow X requests each interval
< gmaxwell>
I can allow more requests, but you're missing my point. It won't be able to stop returning old data then, so it'll send duplicates.
< BlueMatt>
yea, my point was "meh"
< gmaxwell>
then it remains a bandwidth exhaustion attack to keep calling mempool often.
< BlueMatt>
not really
< BlueMatt>
because you limit how often you can call it
< BlueMatt>
to the same rate as your patch
< phantomcircuit>
tulip, ha
< tulip>
the blown up ping?
< phantomcircuit>
i knew that someone would show up having seen that in production
< phantomcircuit>
"production"
< phantomcircuit>
there's lots of things using wall clock time that should be using monotonic time
< phantomcircuit>
but i think for now it's better to explode than to silently do crazy things
< cfields_>
gmaxwell: suggestions for a uuid for seeds rather than a second lookup? best i can come up with is designating a tiny chunk of the FC00::/7 range for groups. I'd say we should just feed addrman a deterministic hash for the source, but the address is serialized to disk
< gmaxwell>
phantomcircuit: I have pointed out before that we should be using monotone time. I don't thinke we have any need for wallclock time, except logging (and even there, if logtimes are non-monotone it will break some log parsing tools for sure)
< gmaxwell>
phantomcircuit: we already have code that does time ofsetting so it can just adjust the offset between the monotone clock and what we want it to be (in a way that preserves monotonicity!)
< gmaxwell>
phantomcircuit: reworking time in bitcoin core is something I've wanted to do since 2011 but its so unimportant that I never will.
< gmaxwell>
cfields_: 127.0.0.2 127.0.0.3 perhaps? :P (I forget if we look at the source netgroup)
< gmaxwell>
even though it gets saved to disk, I don't think we care too much if its not fully consistent across versions. I suppose it's preferable if it is.
< gmaxwell>
phantomcircuit: e.g. I think all Time() in core should be monotone, network time should be largely monotone but with the ability to step if its way out of wack.
< cfields_>
gmaxwell: heh, i figured you'd smack me for not coming up with something more clever than a dummy range. if you're ok with that, i'll go ahead and do it that way and we can bikeshed about what actual range to use in the PR. 127.0.0.x sounds as good as any to me.
< phantomcircuit>
gmaxwell, agreed, but we would definitely want to review everywhere that GetTime* is used before changing their behavior
< phantomcircuit>
but a simple find/replace to GetMonotonicTime
< phantomcircuit>
or maybe just change GetTime to be monotonic and add GetWallclockTime
< phantomcircuit>
which could keep the assert
< phantomcircuit>
either way i think exploding all over the place is the best option for something doable right now today
< gmaxwell>
cfields_: the only downside I see w/ the dummy range is that if we reorder the list it'll behave suboptimally. But this is no great crime. :)
< gmaxwell>
but as I said, if we're currently paying attention to source netgroups (I think we might in one table) them they should probably be in different netgroups.
< cfields_>
gmaxwell: not sure what you mean about reordering?
< gmaxwell>
phantomcircuit: well it's more complex. monotone time is not calibrated to any particular starting point. So that means any place we print one of our stored times we may need to process it.
< tulip>
phantomcircuit: I don't think I would have noticed except that the script the output was being piped to didn't like graphing a ping with 300,000 years of network latency.
< cfields_>
gmaxwell: oh, you mean re-querying with existing addrman data?
< gmaxwell>
cfields_: e.g. if the list is seed1, seed2, seed3 and we change to seed2, seed4, seed1, seed3 in a new release then addresses will go into different groups, perhaps giving one seed or another more share of your addrman than it should. But who cares.
< cfields_>
gmaxwell: roger, and agreed.
< cfields_>
gmaxwell: for background, i'm poking at this in order to make dns async. as-is, it's all tangled up if one resolve has to wait on another.
< cfields_>
so there's more motivation than just "that looks weird"
< gmaxwell>
phantomcircuit: but I think we should return numbers in network time, and then just have a function that converts any absolute time we display to network time.
< gmaxwell>
cfields_: fantastic.
< gmaxwell>
(and I think our log messages should log both network time and local time; /me wants for wumpus to shoot him)
< gmaxwell>
phantomcircuit: actually on most systems the monotone clocks starts at 0 at boot.
< phantomcircuit>
oh
< phantomcircuit>
then *shrug*
< cfields_>
gmaxwell: you'll be happy to know also, the async networking coalesces 4 threads to 1. Not sure how many of those unused threads i get to spend myself, though :)
< * gmaxwell>
looks at x86 virtual address space usage
< gmaxwell>
:(
< gmaxwell>
-3
< gmaxwell>
:)
< gmaxwell>
phantomcircuit: it's no big deal, we already handle network time being some arbritary offset from local time; so instead it would just be network time is an arbritary offset from monotone time, and that offset is initilized with localtime at start.
< * cfields_>
calls shotgun on at least one freed up thread for async block handling of some kind
< gmaxwell>
phantomcircuit: most calculation internally would use monotone time, but display would convert to network time. (and obviously the network time needing things would use network time).
< phantomcircuit>
gmaxwell, oh you mean the adjusted network time thingie?
< gmaxwell>
yup yup.
< phantomcircuit>
hmm yeah that makes sense
< phantomcircuit>
and then finangle that such that it's monotonic unless something crazy happens
< gmaxwell>
well the network time should be monotone except for steps, and we should probably log steps.
< gmaxwell>
but it doesn't need to be, and most of our use of time is differential, and its more important that it be monotone than it have any particular timebase.
< CodeShark>
we have such poor temporal resolution anyhow - ultimately consensus is about well-ordering of events
< gmaxwell>
CodeShark: internally we have high resolution and do varrious things where time going backwards can cause breakage. See tulip's example with the 300 thousand year pingtime.
< CodeShark>
was that part of an earlier discussion here? (scrolling back)
< tulip>
yes, and #7094
< CodeShark>
why were signed ints used for timestamps in the first place?
< CodeShark>
:)
< CodeShark>
what's the 300 thousand year pingtime?
< phantomcircuit>
CodeShark, his clock went backwards or something insane
< phantomcircuit>
oh no i know
< gmaxwell>
CodeShark: because they mostly get used for time differences, which are naturally signed. Not using signed values are part of what creates issues like the 300 years pingtime (instead of negative pingtimes)
< phantomcircuit>
his clock started at 1970-1-1 and jumped to the current time
< phantomcircuit>
or something like that
< gmaxwell>
phantomcircuit: More likely it just went backwards a microsecond, and then the -1 was converted to unsigned at some point before it hit the screen.
< CodeShark>
hah
< phantomcircuit>
so many comical possibilities!
< phantomcircuit>
:)
< phantomcircuit>
gmaxwell, are there systems where the monotonic clock will wrap?
< gmaxwell>
because people have the expectation that time does not flow backwards and write software accordingly.
< phantomcircuit>
i'd like to think the kernel detects that and helps you out but...
< gmaxwell>
phantomcircuit: the monotone clock returned by the OS should be cleaned of any vulgar hardware weirdness.
< gmaxwell>
If it doesn't, we're allowed to fail. :)
< phantomcircuit>
fail catastrophically? "your clock is broken"
< phantomcircuit>
heh
< gmaxwell>
(it's pretty easy to unwrap a monotone clock!)
< phantomcircuit>
gmaxwell, as long as you're polling more frequently than the wrap interval yes
< gmaxwell>
(so long as you know its period and poll it at least twice per cycle )
< tulip>
phantomcircuit: 9223372036854 seconds is 9223372036854000000 microseconds, which is pretty close to 2**64. I think the clock just skipped back further than the ping time.
< phantomcircuit>
yup
< gmaxwell>
independant of this monotone time stuff, we should probably figure out where that stat goes unsigned and make it signed.
< gmaxwell>
or discard negative values before they get that far. :)
< phantomcircuit>
gmaxwell, ok now i can see the need for negative values
< phantomcircuit>
:P
< CodeShark>
why do we care about the other node's clock when doing a ping?
< tulip>
it's the network time thing.
< tulip>
-debug (-debug=net in master) will dump out the deltas of your peers timestamps. spoiler is, they're usually pretty awful.
< gmaxwell>
CodeShark: we don't know _anything_ about the other nodes clock when doing a ping.
< gmaxwell>
CodeShark: it's not the other nodes clock at all.
< gmaxwell>
GetTime() is not monotone. It can go back. Time_ping_returned-Time_ping_sent can be negative.
< CodeShark>
time_t now = time(NULL); - how can that go back unless the user's system clock gets changed?
< tulip>
NTP will change the system clock backwards if it needs to.
< CodeShark>
oh, ok
< CodeShark>
if you only need to measure time intervals, isn't there a better option than time(NULL) that is unaffected by system clock changes?
< gmaxwell>
yes, you ask the system for a monotone clock.
< CodeShark>
is that part of the time.h library?
< CodeShark>
I guess there's the timer stuff
< gmaxwell>
tulip: NTP tries pretty hard to not move the clock backwards, but it will do so if its too far behind... and this can happen like.. constantly, if the path delay asymmetry of the NTP servers you're using changes. (e.g. due to servers going up/down or internet rerouting).
< tulip>
phantomcircuit: RE doing things that rely on the accuracy of peer clocks, here's the offsets from a crawl of about 1000 random IPv4 peers. the data doesn't account for latency so +-300ms is probably within the noise when the nodes are far away. http://pastebin.com/raw.php?i=3zmQDyK0
< CodeShark>
to force monotonicity, could we do something like static int prev = 0; int GetTime() { time_t now = time(NULL); if (now > prev) { prev = now; } return prev; }
< gmaxwell>
sweet now GetTime() needs to take a lock. :P
< CodeShark>
yeah, it's not thread-safe...but we'll pretend we didn't notice ;)
< CodeShark>
how often does it get called, though?
< gmaxwell>
no, not cool. There is no such thing as a safe data race. It's undefined behavior and can cause spooky memory corruption at a distance. :)
< gmaxwell>
CodeShark: all over the place, used for benchmarking stuf, yadda yadda (esp the microseconds version)
< CodeShark>
so then this brings up another issue - even if the system clock is perfectly accurate, things may execute out-of-order
< CodeShark>
I mean, the monotonicity ceases to be meaningful in a concurrent setting
< CodeShark>
in general we cannot say which of two calls on two separate threads occurred first without using a lock
< gmaxwell>
sure but that isn't the case for many things. We do really know that a ping response came after the request. They're seralized by the IO.
< CodeShark>
right
< CodeShark>
in that particular use case, though, a lock doesn't seem like the bottleneck
< CodeShark>
in any case, we can just zero all negative diffs and basically achieve the same goal :)
< tulip>
that would ruin the "minping" statistic
< cfields_>
gmaxwell: speaking of which, I've been considering creating a bip for an explicitly-allowed out-of-order ping/pong. For ex, if the ping nonce is 0xffff, allow that to mean "this is not meant to be a fence, it's an actual latency measurement"
< cfields_>
gmaxwell: as threading improves, we should have a way of specifying that a ping isn't meant to be a fence. Thoughts?
< cfields_>
er, everyone^^. No clue why that was directed at gmaxwell :p
< cfields_>
only one unordered ping would be allowed in-flight at any given time, so dropping the nonce doesn't cost anything
< gmaxwell>
The purpose of the nonce isn't so much for ordering, its to prevent faking a fast response.
< gmaxwell>
I dunno if an unordered ping is actually useful, for every case where I'd like to use pings, I'd like it to go higher if the peer is busy. But if there is a use, I don't have a problem-- but it shouldn't lose the nonce. :)
< gmaxwell>
tulip: yea, thats why I said discard above.
< cfields_>
gmaxwell: well busy can be fleeting.. so it's not a great metric for discerning peering. "Busy" is also kinda an implementation detail. With better threading, a close-by busy peer may be more helpful than a further away idle one.
< cfields_>
re nonce: point taken
< gmaxwell>
cfields_: close by is best measured by the minimum.
< gmaxwell>
(or minimum with periodic forgetting if you're concerned about it changing.)
< cfields_>
gmaxwell: makes sense i guess. assuming you've stuck around long enough for a good minimum :)
< phantomcircuit>
gmaxwell, atomics horray
< phantomcircuit>
compare_and_swap
< Luke-Jr>
gmaxwell: bool races are unsafe?
< phantomcircuit>
Luke-Jr, yes
< phantomcircuit>
Luke-Jr, dont tell ckpool
< phantomcircuit>
it's funnier this way
< Luke-Jr>
what could possibly go wrong in a bool race? :/
< phantomcircuit>
Luke-Jr, overwrite a random byte of memory with garbage?
< Luke-Jr>
that'd be a pointer race?
< phantomcircuit>
Luke-Jr, clever compilers doing clever optimizations have the same effect potentially
< Luke-Jr>
real-world optimizations?
< Luke-Jr>
hm, I suppose the compiler could move a condition check outside a busy loop because it knows the value won't change inside it..
< Luke-Jr>
well, except if for the volatile keyword..
< gmaxwell>
Luke-Jr: any race is unsafe. The compile can do things like "oh, later I will overwrite this variable, that means I have exclusive access to it, I can store temporary thing to it for now."
< Luke-Jr>
hmm, I wouldn't think that is allowed if it's volatile
< gmaxwell>
Volitile should prevent that; though volitile is frequently misimplemented.
< wumpus>
gmaxwell: log messages should log at least current monotonic time, local time, network time, bulletin of atomic scientists' number of minutes to midnight, and the absolute time since the big bang in Planck time units
< gmaxwell>
oh your stabbing is just right.
< gmaxwell>
:)
< wumpus>
gmaxwell: can we have your input on these tests? https://github.com/bitcoin/bitcoin/issues/7086 Looks like one hidden place where openssl is still used, as an oversight, as openssl hasn't been part of consensus in that particular way for ages
< wumpus>
and now the code is running into trouble with newer OpenSSLs
< jonasschnelli>
hmm.... testing coinjoin and QT. Question:
< jonasschnelli>
I have two 50BTC inputs, one from node0, one from node1. I create a transaction with those inputs with two outputs (49.9 to node0, 49.9 to node1). What would be expected as output for listtransactions and over the GUI?
< jonasschnelli>
I expect only the relevant in/outs should be listed/calculated for the net?
< gmaxwell>
It displays the coins that weren't yours as negative fee.
< gmaxwell>
Which is ... internally consistent though a little confusing when you're not expecting it.
< wumpus>
ah yes the negative fees when you don't own all inputs, that's not just a qt issue
< jonasschnelli>
Yes. Listtransactions tell me now: "fee": 49.98000000
< gmaxwell>
It's logical in a sense, but hah. can be a real heart stopper.
< jonasschnelli>
haha... right.
< wumpus>
hehe indeed
< gmaxwell>
FWIW, if I had champaign or really drank, I would have poped a cork at that issue being opened.
< gmaxwell>
This is the first real proof of coinjoin being in widespread use by actual users: the reported a known 'bug'!
< jonasschnelli>
Indeed. At it was reported by two users (independent) within 1.5 month...
< wumpus>
it is good to see people actually using coinjoin, absolutely!
< gmaxwell>
(bug just in quotes because I'm not actually sure what our behavior should be. Without access to the inputs in the wallet, it's hard to handle sanely!)
< gmaxwell>
(reporting it all as fee might actually be the best we can do with the data available)
< wumpus>
can we 'see' that something fishy is happening and flag the transaction as 'probably coinjoin',?
< wumpus>
that'd at least help with the UI aspect a bit
< gmaxwell>
yes, well if there are inputs from outside the wallet we can just flag it as "used outside funds"
< gmaxwell>
Or "used outside funds (coinjoin? lighthouse?)"
< wumpus>
e.g. "you may be seeing exorbitant fees on this transaction because it involves foreign inputs"
< wumpus>
right
< gmaxwell>
well they're the opposite of exorbitant. :P
< wumpus>
negative exorbitant :p
< gmaxwell>
"exuberant fees"
< wumpus>
people may not see the sign correctly, but agreed
< wumpus>
LOL!
< gmaxwell>
yea, it freaked me out the first time.
< gmaxwell>
well actually not the first but the first time I did a CJ with an ordinary amount of coin.
< gmaxwell>
I wasn't likely to mistake this for a too high fee error:
< gmaxwell>
"fee": 50000.31337000,
< gmaxwell>
(this is not output from a testnet wallet)
< wumpus>
whoa
< gmaxwell>
but later a 10BTC one startled me I think.
< wumpus>
was that the 'link yourself rich' or something transaction from bitcoin talk?
< gmaxwell>
It works, FWIW, lots of wallet tracker things think that address is all kinds of stuff.
< gmaxwell>
well half-works, they were supposted to notice all the linked up stuff and realize that what they were doing was dumb.
< wumpus>
it's promising for the future at least when it is more used, they probably won't adjust dumb behavior for one transaction :)
< gmaxwell>
well I made lots of coinjoins with that address, with lots of other people who went on to do coinjoins, and also joined with keys and got them imported varrious places.
< gmaxwell>
otoh I now have to avoid anyone donating to it, because I probably can't spend any coins sent there in a lot of places. :(
< gmaxwell>
in any case, it seems to think that 'wallet' has 266 addresses, and I believe only two of them are mine.
< gmaxwell>
actually checking, three of them, how'd that one get linked. :-/
< CodeShark>
expecting dumb people to notice that what they're doing is dumb is a risky success strategy - and avoiding donations is a good kind of problem to have :)
< tulip>
has anybody tested -blocksonly with dynamic fees?
< tulip>
I sort of expected the dynamic fee RPC calls to return -1, but they don't. they're returning values.
< gmaxwell>
tulip: I have, results in the -1 all the time (unless you have prexisting save predictioned), in which case they just fade out.
< gmaxwell>
the results surprised me too, but they go away if you delete the saved stats file.
< tulip>
ah good call.
< tulip>
maybe they should just be hard wired to -1 with blocksonly?
< gmaxwell>
tulip: so blocksonly isn't necessarily all blocks only; as you can have whitelisted peers you accept txn from. I was also contemplating a rpc command to one shot perform a top mempool fetch from a single peer; but I dunno if we'd take that upstream.
< gmaxwell>
but perhaps it should be hardwired -1 when the mempool is totally empty?
< tulip>
right, ok.
< gmaxwell>
the goal with the saved state is to give useful resutlts as soon as you start.
< tulip>
I'd completely forgot it exists, that's all.
< GitHub102>
bitcoin/master 392d3c5 Cory Fields: build: Set osx permissions in the dmg to make Gatekeeper happy
< GitHub102>
bitcoin/master 348b281 Wladimir J. van der Laan: Merge pull request #7092...
< GitHub54>
[bitcoin] laanwj closed pull request #7092: build: Set osx permissions in the dmg to make Gatekeeper happy (master...osx-perm-fix) https://github.com/bitcoin/bitcoin/pull/7092
< wumpus>
jonasschnelli: good call on closing #7089 - indeed, bitcoin's core github is not the place to request changes to consensus behavior
< jonasschnelli>
wumpus: Yes. It was a bit harsh but I think we need to keep discussions out of the github context.
< gmaxwell>
hm. I didn't even see #7089 but yes.
< gmaxwell>
Kind of irritating that they keep going back to probably the worst proposal for that ever done. (e.g. needlessly a hardfork)
< jonasschnelli>
But i agree with him. It causes big pain for HWW devs to trustworthy validate fees (=input amounts).
< gmaxwell>
Oh absolutely.
< gmaxwell>
CHECKSIG in elements alpha tries one fix, though a different one would be better. I expect there will be a decent soft fork new checksig operator proposed within sig months.
< jonasschnelli>
Not sure what is best practice there. But I think retrieving over SPV/BF the inputs and validate these together with the transaction is the only feasible way.
< gmaxwell>
jonasschnelli: Currently, sure. But if we add a new checksig operator it can avoid having to check the inputs at all.
< wumpus>
providing the dependent transactions together with the transaction seems like the only feasible way, certainly if the HW wants to verify that the inputs are really the ones given
< gmaxwell>
Checksig already signs the txin scriptpubkey but what it really should sign is the whole utxo. If it did, then when signing you need only provide the utxos and not the whole parent transactions. If you lie the signature will be invalid.
< gmaxwell>
But this can't be done as a sighash flag to checksig. Well it can but it would be insane and irresponsbile to hardfork the network for that.
< wumpus>
indeed it signs the scriptpubkey, and that doesn't help verifying the amount
< wumpus>
sure, a hardfork for that is stupid
< tulip>
it looks like there's mining software on the main network which is rolling the sequence number. not disastrous or particularly interesting at the moment, but it might be notable if anybody is altering the behaviour of sequence numbers with a consensus rule like in BIP112.
< gmaxwell>
lol oh boy. fortunately BIP constrains the transaction version number.
< gmaxwell>
er that BIP.
< gmaxwell>
tulip: thanks for noticing that. :(
< tulip>
might be problematic if miners using this software just flip forward the version number though.
< wumpus>
flipping forward the version number like people click 'OK' on software's terms and conditions
< jonasschnelli>
hmm... testing wallet.py over the GUI makes the test fail.
< jonasschnelli>
It looks like that self.nodes[2].settxfee(Decimal('0.001')) has no effect
< sipa>
so it makes sense that from without coincontrol you're able to set the absolute fee
< sipa>
but it should 1) use a separate variable (not paytxfee, but for example overridetotalfee) 2) it should be passed down through the coin control preferences, not through a global
< jonasschnelli>
Right. It should not be a global variable and should not touch outside GUI behavior. It's actually also available without CoinControl.
< sipa>
well it being set to true by default is broken
< sipa>
and it can only be unset through the coincontrol gui
< jonasschnelli>
okay. will work on a fix.
< jonasschnelli>
nFeeNeeded = payTxFee.GetFeePerK(); // <--- is a dirty hack!
< jonasschnelli>
but i see, that an absolute fee can help (during RPC tests and maybe some other usecases).
< jonasschnelli>
What about adding an option to set absolute fees over "settxfee"?
< sipa>
makes no sense
< sipa>
without coin control
< jonasschnelli>
It only makes sense for regression tests.
< sipa>
and if you're doing selection manually (via createrawtransaction), there is no need for it
< jonasschnelli>
to deterministic calculate fees
< sipa>
well if it's exposed by RPC it is necessarily global
< sipa>
(for RPC)
< sipa>
let's first fix the use case, then the rest?
< MarcoFalke>
Didn't catch that, so we are not duplicating effort
< jonasschnelli>
MarcoFalke: I think sipa has just started to work on some things there. I'll wait until he has something.
< jonasschnelli>
Then i'd like to address the UI stuff
< sipa>
jonasschnelli: so my suggestion would be to remove the "pay at least total" function from the regular send dialog, and move it to the coin control one
< sipa>
then the setting can just be passed via the coincontrol settings object, which is trivial
< sipa>
MarcoFalke: no, to fix the fact that the "set at least total" function works via a global and changes all fees
< sipa>
why was this introduced?
< jonasschnelli>
sipa: right. It should be in the CC dialog.
< jonasschnelli>
<sipa>why was this introduced? / you mean the "pay at least total"? Probably together with Cozzes CC stuff?
< MarcoFalke>
Yes it's by cozz
< sipa>
so nobody ever noticed that this makes bitcoin core's transaction pay around 4x too much fee by default?
< jonasschnelli>
sipa: hah. I assume "no"!.. scary!
< sipa>
oh, i assume paytxfee was 0 by default, so it didn't have any effect
< * jonasschnelli>
thinks Cozz was payed by the miners... :)
< jonasschnelli>
sipa: right. This is a point.
< sipa>
jonasschnelli: see my betterabsolute branch
< sipa>
jonasschnelli: sorry, i thought i could do more, but there rest is gui code
< MarcoFalke>
I haven't checked previous code but it may be to mimick how earlier version have done that.
< MarcoFalke>
Was this "let's use round numbers for fees" a thing back then?
< sipa>
right, i think until some time in the past, we always paid per kilobyte started
< jonasschnelli>
sipa: np. I'm happy to take over.
< sipa>
not per byte
< sipa>
jonasschnelli: thanks!
< MarcoFalke>
sipa, should I rebase onto betterabsolute?
< MarcoFalke>
or wait until the gui is fixed
< sipa>
MarcoFalke: up to jonasschnelli
< jonasschnelli>
MarcoFalke: let me finish it, i'll also cherry-pick your wallet.py changes,... maybe afterwards there are still things that could be optimized.
< wumpus>
it never was; gitian-downloader was meant to be a downloader for bitcoin that checks the gitian signatures, but AFAIK it was never completed or used
< MarcoFalke>
But the folder is used to store the pgp keys?
< wumpus>
yes, just a convention
< wumpus>
the download-configs are pointless
< wumpus>
I think they've been kept around in case anyone was ever going to resurrect the tool, but that never happened, I guess they're out of date enough to be removed now
< MarcoFalke>
Just doing this with my next trvial pull
< MarcoFalke>
Oh, that's why travis is only running with 3/5 speed.
< cfields_>
not sure what the reasoning was for the double lookup, does it serve a purpose that a static dummy source wouldn't?
< cfields_>
ok, i must be going crazy... using a vanilla node with no externally bound address, ipv4/ipv6 aren't set as reachable by default. So when addr's come in, they're not added to addrman since they fail IsReachable(). Result is that addrman only ever contains dns seeds
< cfields_>
...that can't be right.
< gmaxwell>
oh that bug. damn
< gmaxwell>
I think phantomcircuit (?) found that a couple months ago, someone did, and I forgot about it. So it may well be true.
< cfields_>
gmaxwell: so that's actually the case? whoa.
< cfields_>
just verified with an addrman dump
< cfields_>
gmaxwell: surely it's reasonable to set a net to reachable once a connection has been established on it?
< gmaxwell>
I think we should be setting IPv4 reachable if we have any IPv4 bindings at least.
< gmaxwell>
and tor reachable if we have a onion proxy configured.
< cfields_>
i believe the latter is already done
< gmaxwell>
wumpus: I switched to a somewhat different approach in that mempool limit patch. I now have it set setInventoryKnown (I think it should have always done this), and not return anything in setInventoryKnown. It also only considers the top 8192 entries in the mempool.
< GitHub82>
[bitcoin] gmaxwell opened pull request #7099: Add whitelistforcerelay setting and default to off. (master...control_relay_force) https://github.com/bitcoin/bitcoin/pull/7099