< tulip>
maybe there's something between you and his DNS server which is mutating it?
< cfields_>
(assuming that's how you queried it, ofc)
< tulip>
yes.
< cfields_>
hmm. libevent turns on a check by default, so resolves in my test app are failing. when i turn the option off they succeed. "dig" succeeds as well.
< cfields_>
i'll keep poking around.
< cfields_>
(matt/jonas seeds work fine)
< BlueMatt>
wumpus: fyi, if you want me to see something on github, tag @BugTheBlueMatt not @TheBlueMatt (which just gets delivered just like any other notification email)
< BlueMatt>
(and there doesnt seem to be a way to have github route to a different email address if you're tagged :/ )
< sipa>
BlueMatt: disable all notification mails except tagging :)
< BlueMatt>
sipa: but I dont want to do that :(
< BlueMatt>
wumpus: also, wait, why does the gitian build env need network access?
< BlueMatt>
it really shouldnt be allowed to have network access (we really want to support doing builds via gitian on airgapped machines)
< BlueMatt>
oh, the pr is still open, i'll go comment there
< dcousens>
what happens if SIGHASH_SINGLE is used, but no corresponding output exists?
< tulip>
the SIGHASH_SINGLE bug :P
< dcousens>
haha, nvm, thats right
< sipa>
dcousens: hash == 1 is signed
< midnightmagic>
BlueMatt: it is possible to do it entirely offline unless something fundamental changed.
< gmaxwell>
Fishing for concept acks dgenr8 pointed out that the sendbuffer size reduction a long time ago ended up turning the setInventoryKnown mruset down to 1000 entries! I'd like to change that mruset to do per-peer salted key hashing and truncate to 64-bits, then increase its size to 100,000 entries. With 64 bits the probablity that there has ever been a collission in the history of all transaction
< gmaxwell>
s is only about 0.02% and even if there were one, it would just mean that you'd fail to INV a transaction to a single peer.
< tulip>
height 385363 has 521 txns, 1142 sigs, 169 cache misses. not bad for a node with 10 minutes of uptime.
< gmaxwell>
tulip: may be due to a peer spamming you with their mempool at connect.
< tulip>
maybe.
< sipa>
gmaxwell: or a rolling bloom filter?
< gmaxwell>
I was trying to keep MRU behavior, but maybe we don't really need it.
< gmaxwell>
would certantly be easier.
< gmaxwell>
BlueMatt: on that subject, I still have this behavior where RNC seems broken; -- it's my only source of transactions on a testnode and I've never seen it get more than 1671 tx.
< gmaxwell>
okay jinx, it's actually at 1783, but thats still small.
< sipa>
gmaxwell: rolling bloom filter is effectively mru
< gmaxwell>
RB is not per-use salted? weird. why.
< gmaxwell>
what. I am boggle this code.
< gmaxwell>
oh I see.
< gmaxwell>
still, don't see why it's not salted.
< sipa>
it is salted?
< sipa>
no?
< gmaxwell>
constructor has no salt argument and calls the underlying blooms with a tweak of 0.
< gmaxwell>
looks like you have to call reset to randomize it.
< sipa>
oh, right, we didn't want to call getrandbytes in a global constructor
< gmaxwell>
ah duh.
< sipa>
but it could actually automatically reset upon first use
< gmaxwell>
just making ninsertions oversize would do that. Is there a reason to just not reset it every time a table is changed?
< gmaxwell>
e.g. with b1/b2 are cleared?
< sipa>
yes, that's what i'm suggesting
< gmaxwell>
No reason I can see why the two should have the same tweak.
< sipa>
but only when an actual entry is written to it
< sipa>
not on construction
< gmaxwell>
yea, sure, I mean make the clear functions in insert do it when they clear.
< sipa>
right
< gmaxwell>
whats with the 2* in the rolling bloom filter?
< sipa>
it's needed
< sipa>
:)
< gmaxwell>
whats an acceptable FP rate for the known filtering?
< sipa>
you need around 4.8 bits per element per 1/10 FP rate
< sipa>
so 6*4.8 bits per element for one in a million
< dcousens>
"what. I am boggle this code." lol
< gmaxwell>
sipa: as far as 'mru', well-- so it doesn't reinsert in the other filter when it hits. And doing it yourself isn't quite ideal since you'll increment the count.
< sipa>
?
< gmaxwell>
so I think the contains code should check one filter, but insert in the other, not yet mature filter, in order to have the behavior you expected.
< gmaxwell>
if I send you the same item non-stop, and you drop it when contains, it will fall out.
< sipa>
right
< sipa>
it is most recently added, not most recently used
< cfields_>
Luke-Jr / sipa: nevermind the dns red herring earlier. It looks to be a combination of a libevent implementation detail and a dns resolver implementation detail. annoying.
< cfields_>
we bump up against the 512 byte udp limit. Due to compression, messing with CaSe can grow the payload. Different resolvers/cachers react in different ways when hitting the 512 byte mark.
< cfields_>
In this case, my local cache appends additional records. Apparently smart resolvers strip those off and just ignore the truncation. however, libevent marks a truncation as a failure.
< GitHub92>
[bitcoin] gmaxwell opened pull request #7100: Replace setInventoryKnown with a rolling bloom filter. (master...known_bloom) https://github.com/bitcoin/bitcoin/pull/7100
< jonasschnelli>
CoinJoin: in case of a coin-join wtx. What if we show one table-row for the self-owned in/outs net, and table-rows for each "normal-output" (non self),..
< wump>
so re: https://github.com/bitcoin/bitcoin/pull/7096 was this change in RPC settxfee behavior ever planned, or noticed before? I think it's strange that this is in 0.11, but no one reported the change from per-kB to absolute fee?
< jonasschnelli>
and because the debit-credit != 0, add another table row with the combined input sum of all foreign-inputs (something like "not-self-owned-funds")?
< gmaxwell>
wump: ?? when was something changed to absolute fee?
< wump>
jonasschnelli: this would break one table row = one output
< wump>
gmaxwell: that was my surprise as well
< jonasschnelli>
wump: settxfee: I think it went from "perKB" to "absolute" with the coin-control smart fee changes.
< jonasschnelli>
5200 was the problematic PR that made settxfee work absolute instead of byKB
< gmaxwell>
also, I think we need to write some polite admonishment for test authors to please try to not just hardcode bugs into the tests. This has happened several times now, and I don't know that people have a good theory of testing that helps them not do that.
< gmaxwell>
as far as people not complaining, mixture of many txn being <1kb, and fee noise on the network.. and maybe smart fees making mysterious behavior expected?
< wumpus>
jonasschnelli: but this affect the non-UI as well right? I'm much more concerned with that
< jonasschnelli>
right
< jonasschnelli>
This is the problem: nFeeNeeded = payTxFee.GetFeePerK();
< jonasschnelli>
We take a perKB fee rate as absolute fee.
< wumpus>
I mean the UI has zillion options to configure the fee before sending, the default is just to use smartfees, and everyone almost uses that
< jonasschnelli>
But only when fPayAtLeastCustomFee = true
< jonasschnelli>
(which is the default!)
< wumpus>
but I'm not sure about RPC...
< jonasschnelli>
IMO it's bad that the UI can change global state during "normal operation" (non-settings operations)
< wumpus>
how are settxfee and smartfees *supposed* to interact?
< wumpus>
jonasschnelli: I agree
< jonasschnelli>
The 5200PR included changes that allowed UI users to set an absolute fee (which makes sense if you use coincontrol). Somehow this is unrelated to smartfees.
< jonasschnelli>
a) absolute fees should only be possible when enabling and using coincontrol
< wumpus>
there should be nothing with absolute fees IMO
< jonasschnelli>
b) UI set absolute fees should never change the global fee level!
< wumpus>
fees should be relative througout bitcoind
< gmaxwell>
coincointrol should still work with relative fees, but it knows the transaction size and so it can show the actual amounts.
< jonasschnelli>
right.. the PR 7096 does minimize the possibilities of absolute fees.
< wumpus>
not sure how this sneaked in, but this is the kinds of things I've always been afraid of with wallet changes. They're hardly reviewed/tested.
< wumpus>
but I'd like to know: how are settxfee and smartfees *supposed* to interact?
< gmaxwell>
well no harm no foul at least.
< gmaxwell>
If we don't have any wallet users at all there will be no bugs. :)
< jonasschnelli>
simply spoken: settxfee has nothing to do with the "bug".
< jonasschnelli>
The problem lays deeper in "CWallet::GetMinimumFee"
< wumpus>
gmaxwell: right, no one reported this, but it may explain some 'my transaction never goes through'
< wumpus>
jonasschnelli: right, maybe it has nothing to do with that, to be honest I don't understand the current tangle of rules determining the fees for RPC
< jonasschnelli>
There is no way to change this unsless you use the UI
< jonasschnelli>
fPayAtLeastCustomFee is always true
< gmaxwell>
wumpus: on tests, generally tests that are trying to sense if nothing changes at all should be seperated from ones checking important invarients. Many of our tests don't test invarients at all but only check exact values; and it makes it very hard to change anything.
< gmaxwell>
and when you do there is a temptation to just change the fixed values; which moots the value of the test.
< wumpus>
gmaxwell: I agree, but I don't really see a solution to that, that's been the case in every project I've been in that even had tests :)
< wumpus>
oh the tests break? change the tests
< raedah>
jonasschnelli, re coinjoin, "table-rows for each 'normal-output' (non self),.." that would be a lot of rows for a single big join. I can show the just self outputs easily, alongside a row for the Net.
< wumpus>
jonasschnelli: ok. Let's just remove anything to do with absolute fee setting.
< jonasschnelli>
wumpus: PR 7096 does that "almost".
< jonasschnelli>
It enables the option to set an "absolute fee" IF coin join is enabled and inputs are selected.
< jonasschnelli>
I think that's okay.
< wumpus>
gmaxwell: I'm sure better tests are always welcome though, and you're welcome to encourage people to add better tests :)
< jonasschnelli>
The "absolute fee" option is even hidden if coinjoin is disabled.
< jonasschnelli>
s/coinjoin/coincontrol
< gmaxwell>
wumpus: whenever someone creates a test there should be at least three test sets: An invarient test which any possibly correct code should pass. A known response test that might need to change if the code changes, and an extreme values test, which may or may not be implementation specific. Yadda yadda. Thinking this way usually makes it easier to write tests, since you can just work through the
< gmaxwell>
cases.
< gmaxwell>
jonasschnelli: if so I think the UI should just be translating that to relative fees
< wumpus>
also we have lots of randomized algorithms, and randomized tests
< jonasschnelli>
raedah: right. I just think we need to have a coinjoin solution where the sum = 0. SOmehow i think if you sum the balance of each row it should be equal to your balance
< jonasschnelli>
gmaxwell: agree, translating to a perKB rate would be nice. Are there no use cases where manual transaction creation (with coincontol) could require a absulte fee?
< wumpus>
it's pseudo-randomized using the hash function, but exact results are tested
< gmaxwell>
jonasschnelli: AFAIK nothing about the bitcoin system uses absolute fees, and it would be irrational for miners to do so.
< wumpus>
so it's very attractive now to just use a hammer and make sure the function is the same on all platforms, but in principle this is a test problem
< raedah>
jonasschnelli, i think thats impossible. You have an input tx and then two output txs that are all yours. The two outputs tx are worth slightly more (or less) then your input. To make matters worth, no where in the transaction does it say what your individual contribution to the miner fee was.
< jonasschnelli>
gmaxwell: but right now, the goal should be to fix the behavior with a minimal impact while not removing current features (so we can backport).
< gmaxwell>
wumpus: I think it's okay to have known response tests like that; but they should always come with invarient tests, so if only the known response test fails you can update it with more confidence.
< jonasschnelli>
but we could also remove the absolute fee entirely...
< gmaxwell>
raedah: You actually cannot tell your contribution to the fees, it's unknowable to the wallet.
< wumpus>
jonasschnelli: I still think that's best - if you know the size of the transaction, you can achieve the same using relative fee
< wumpus>
jonasschnelli: (and the only case in which absolute fees are useful are *when* you know the size of the transaction anyway)
< jonasschnelli>
wumpus: or if you don't trust the fee estimator?
< wumpus>
jonasschnelli: you can still provide a fee per kB right?
< gmaxwell>
I think removing a feature in a backport can be less of a sin than running different code in master and the backport.
< jonasschnelli>
wumpus: right...
< wumpus>
jonasschnelli: no fee estimator involved
< gmaxwell>
Simply because the backport will not be as extensively tested.
< wumpus>
'fee estimator' has to be selected (that's smart fee)
< wumpus>
well the requirement for backport are: small code impact, easy to test
< jonasschnelli>
we could try to get https://github.com/bitcoin/bitcoin/pull/7096 reviewed and merged, and backported.... then remove the absolute fee from master (which is trivial)
< wumpus>
if there is one line of code that fixes 'settxfee' behavior in 0.11 without touching anything else I'd e.g. prefer that
< jonasschnelli>
I could try to see if the code-change is less invasive if we remove the absolute fee. Guts tells me it could be a larger diff.
< gmaxwell>
raedah: only if the wallet knew which outputs were yours (it can't because coinjoin), AND knew the values of the foreign inputs (it doesn't because of the wallet design) could it know your contribution to the fee.
< wumpus>
jonasschnelli: right...
< wumpus>
jonasschnelli: doesn't seem like a large diff anyway, probably applies cleanly right now?
< gmaxwell>
yea, we could do the smallest fix that fixes settxfee, then later remove the absolute in the gui.
< jonasschnelli>
and a backport should be easy because we didn't changes that part since then i guesx
< gmaxwell>
wumpus: wrt reconnecting; in my nodes for what became the peer eviction logic one of the criteria I'd mused over was tracking how fast peers reconnect and prioritizing evicting fast reconnectors.
< wumpus>
gmaxwell: yes - nodes that reconnect at all are suspect. They're either your own or a spy node :)
< wumpus>
so if you whitelist your own nodes, that leaves spies
< gmaxwell>
wumpus: yea, I didn't go much further there because I started thinking that spies are better handled by starvation: stop leaking data thats useful to spies.
< wumpus>
gmaxwell: sure...
< wumpus>
as long as they don't do anything resource intensive, or there are that many of them that they block out signiicant numbers of legit nodes
< jonasschnelli>
raedah gmaxwell: coinjoin: I had in mind to visually flag all coinjoin outputs in the table (different background, different icon, additional icon). And maybe a row that would represent the sum of non-self-controller inputs... but i agree with wumpus, would be a concept change.
< jonasschnelli>
Somehow it looks strange if the sum of all you row in the transaction-table != your balance.
< jonasschnelli>
that's why i had this idea in mind.
< raedah>
gmaxwell, which outputs are mine are known because they are watch addresses, and from that I can deduce which ones are not mine, but the fee contribution happens when the transaction is being assembled and only the fee total ends up in the blockchain. Of course the fee record is in the joinmarket logs, but that doesnt help QT transaction details.
< wumpus>
jonasschnelli: I certainly agree with visually distinguishing coinjoin transactions, and warning that the fee is not really the fee
< gmaxwell>
wumpus: unfortunately it only takes a couple people trying to connect to everything that its intrusive; but my hope was that if they gained nothing they'd mostly stop.
< jonasschnelli>
wumpus: is displaying the fee (which is not known at this point) even required?
< wumpus>
jonasschnelli: but personally I'm not a huge fan of the idea of adding more rows for them
< gmaxwell>
raedah: by outputs I mean what the transaction is paying. It's certantly possible to use JM in a way where you are not paying yourself.
< wumpus>
jonasschnelli: consider this: what if every transaction you do is coinjoin
< wumpus>
jonasschnelli: (I somewhat hope that is going to be the future)
< jonasschnelli>
Or we could try to combine them in one row and add an extra "coinjoin" info in the tx-detail window? overkill?
< gmaxwell>
jonasschnelli: just remember that external inputs are not only made by coinjoins; we should probably handle transactions with unknown inputs in a uniform way.
< wumpus>
jonasschnelli: there shouldn't be too much overhead in that case
< gmaxwell>
Really robust handling requires extra metadata: you need to know which of the payments are yours.
< wumpus>
yes
< jonasschnelli>
okay... so what would be the way to go? :)
< gmaxwell>
We should probably change the 'wallet equation' from myinputs - fee = outputs to myinputs + otherinputs - fees = outputs.
< wumpus>
but we don't have that metadata right now, so we have to handle this without it
< gmaxwell>
And to ever transaction in list transactions add an extra_inputs: field, and get the fees right.
< gmaxwell>
but this requires adding information about utxo not currently in the wallet.
< gmaxwell>
That would be a "level 1" handling, "level 2" would extend that by learning from JM, etc. which outputs were ours.
< gmaxwell>
and the others could be hidden like change is now.
< gmaxwell>
Right now we have a pretty strong assumption that we have full transactions, which makes pruning the wallet hard. E.g. you can't forget old transactions without losing track of spendable utxo or making the non-forgotten transactions show crazy data.
< wumpus>
right, the wallet needs additional information about inputs
< wumpus>
so that even if the originating transactions are no longer known, it still knows the sizes involved
< raedah>
gmaxwell, im not clear here. We do know what our input and outputs in the transaction are.
< wumpus>
raedah: we don't know what our outputs are, I think?
< raedah>
all inputs and outputs are loaded as watchaddresses
< wumpus>
at least in the case of an outgoing coinjoin transaction we can't distinguish what we intended to send from what the others sent
< gmaxwell>
raedah: we don't; we know some inputs came from our wallet, we see other inputs but do not know their values (it's knowable, but the wallet doesn't track it). Then there are outputs, and unless you are paying yourself, the wallet doesn't know which are 'yours'.
< gmaxwell>
raedah: well thats busted.
< wumpus>
where? why would you load outputs as watchaddress?
< gmaxwell>
loading them all as watch addresses still doesn't tell you which are yours, it just make it look like they all are. :)
< raedah>
not all the addresses in the transaction are watch addresses. Just the ones in your joinmarket wallet become watch addresses.
< wumpus>
and loading the inputs as watch-only doesn't make sense either, they're still not your inputs or your balance
< gmaxwell>
raedah: okay, then again the wallet doesn't know the values of coins from other users; and it doesn't know which payments are yours unless you are paying yourself.
< gmaxwell>
a JM maker is all paying themselves, true, but not a JM taker.
< wumpus>
I'm not sure how watch-only helps here at all
< gmaxwell>
wumpus: JM's 'market makers' have their own external wallet managed by JM, effectively.
< wumpus>
ok - if there is an external wallet owned by you adding *those* as watchonly makes sense
< wumpus>
because it's your balance. But adding other people's inputs/outputs would not be a good idea.
< gmaxwell>
Basically JM's coinjoin 'ecosystem' implements an asymetric design where there are people hanging around who put up coin for people to join with ('market makers'); and then users can show up and pick up join bids from one or more makers to construct a coinjoin payment.
< raedah>
wumpus, and joinmarket requires it to pull the data it needs on those addressses from the rpc.
< gmaxwell>
The makers just deposit some coins into JM and it produces income (though not much because there is lots of competition!)
< wumpus>
gmaxwell: makes sense
< gmaxwell>
raedah: do you know specifically what it needs?
< gmaxwell>
If it need to look up txouts, that can be done without wallet importing; we have an rpc for that.
< raedah>
i think its looking for address balances, and maybe tx confirmations.
< * gmaxwell>
ducks into #joinmarket
< wumpus>
both can be determinied using just the utxo set
< wumpus>
(as anything actively spendable, is, by definition, in there)
< wumpus>
no need for any scan over transaction history
< * wumpus>
tries to remember how the RPC call to query utxos was called...
< gmaxwell>
They may not know about gettxout or gettxoutproof.
< wumpus>
heh :-)
< gmaxwell>
earlier on it used some third party API; which probably didn't provide an interface intended for scalablity. :)
< wumpus>
(gettxoutproof is also documented in the same place)
< wumpus>
does that give enough functionality or do you really need to query the utxo set by address/output script?
< gmaxwell>
It shouldn't!, but I'm asking.
< gmaxwell>
after all, you're talking to all the participants, so they can identify their coins. But maybe they put some dumb expectation into the protocol based around using an API.
< wumpus>
(adding an RPC for that was considered at some point, without index it'd be kind of slow, but OTOH much faster and more efficient than a rescan)
< gmaxwell>
(if so, then that can be fixed.. as the protocol is far from set in stone)
< gmaxwell>
When andytoshi's back online he should be pinged to differentially test against that.
< gmaxwell>
He reimplementd univale in rust and has a testharness that checks if it round trip reseralizes the same as univale; and that was one of the cases he has to filter out.
< wumpus>
doesn't rust already have libraries for json parsing/generation? or was that mostly an educational exercise?
< jonasschnelli>
wumpus: do you think fixing the settxfee bug with a tiny risk of RPC/UI interfering is okay? IMO its okay.
< jonasschnelli>
I don't expect people using RPC and UI at the same time.
< jonasschnelli>
Maybe we should warn in the release notes at least.
< wumpus>
jonasschnelli: well, we should try to rule out that risk as much as possible
< wumpus>
jonasschnelli: sure, as long as the conditions under which it happens are clear, and it's an unlikely scenario, warning about it in the release notes would be enough
< wumpus>
but 'using RPC with the UI randomly crashes or sends random amounts to random people' would not be :)
< jonasschnelli>
Right... I don't expect people to blame us of not fixing the UI/RPC interfering bug over backports.
< MarcoFalke>
About fees and release notes: I think a general write up about how fees work now should be planned
< wumpus>
MarcoFalke: yes that would be welcome
< jonasschnelli>
I think we do need to backport the rpc test as well? We don't want to have broken tests in 0.11?
< wumpus>
although it's still kind of in flux, e.g. the final shape of mempool limiting isn't entirely clear yet
< * jonasschnelli>
is working on the most simplest backport
< wumpus>
jonasschnelli: yes, definitely
< wumpus>
jonasschnelli: tests are the most important part, they prove that it's fixed :)
< jonasschnelli>
wumpus: need to bother you with a UI questions: why did you implement the UI block updates (num blocks, last block date) with a timer function with a TRY_LOCK?
< jonasschnelli>
would something speak against using the signal?
< jonasschnelli>
the timer is still required for the bandwith graph...
< sipa>
jonasschnelli: the signal may fire way too frequently sometimes, afaik
< sipa>
up to the point that updating the GUI was slowing down synchronization
< jonasschnelli>
the timer with the trylock fires every 250ms!
< sipa>
during IBD the signal may fire every ms :)
< jonasschnelli>
okay... the UI could measure the time delta (inexpansive uint64_t compare) and update the ui every 5 sec or so.
< jonasschnelli>
it just looks like that the TRY_LOCK often can acquire the lock
< jonasschnelli>
can't
< jonasschnelli>
sipa: if i recall that correctly, the signals `CBlockIndex *pindexNewTip` does not require a lock... so the UI update could be done on the UI tread?!
< sipa>
jonasschnelli: that's correct, but signals still run synchronously
< jonasschnelli>
sipa: right,.. but at least the could emit/pass a QT async signal to the GUI thread
< GitHub0>
[bitcoin] pstratem opened pull request #7104: [Mining] Build empty block on when chainTip changes. (master...2015-11-26-gbt-latency) https://github.com/bitcoin/bitcoin/pull/7104
< sipa>
jonasschnelli: i don't think you want to emit 1000 signals per second still :)
< jonasschnelli>
sipa: hmm... let me benchmark it,.. maybe a min-time-delta is required to avoid over-updating the ui.
< jonasschnelli>
and the signal only fires !fInitialDownload
< sipa>
ah
< jgarzik>
RE univalue - I see one outstanding issue to address that wumpus ping'd on - everything else is fixed in upstream univalue tree.
< jonasschnelli>
sipa: is "Tip Notification: 0.03ms" reasonable (standard core i7 laptop)? Currently I update the UI also during fInitialSync.... looks nice!
< sipa>
that seems ok...
< jtimon>
cfields_: I can't undesrtand why these commits break the build, it would be great if you could explain. why does the order matter in bitcoind_LDADD? why crypto/libbitcoin_crypto.a works but consensus/libbitcoin_consensus.a doesn't (inside of the consensus dir)? https://github.com/jtimon/bitcoin/compare/consensus-build...jtimon:consensus-build-full
< sipa>
jonasschnelli: going to try to implement another approach for 7067
< jonasschnelli>
sipa: that was actually my intention... :)
< jtimon>
cfields_: I mean, the first commit in #7091 builds locally (xubuntu14) but travis says it already breaks the build for everything but mac, but maybe those errors I get locally and can't uncerstand are related
< GitHub6>
[bitcoin] sipa opened pull request #7105: Keep track of explicit wallet conflicts instead of using mempool (master...realconflicts) https://github.com/bitcoin/bitcoin/pull/7105