1 2016-12-04 08:25:01 0|bitcoin-git|[13bitcoin] 15TheBlueMatt opened pull request #9273: Remove unused CDiskBlockPos* argument from ProcessNewBlock (06master...062016-12-remove-unused-pnb-param) 02 https://github.com/bitcoin/bitcoin/pull/9273
2 2016-12-04 18:24:55 0|bitcoin-git|[13bitcoin] 15MarcoFalke opened pull request #9274: [qa] pruning: Use cached utxo set to run faster (06master...06Mf1612-qaPruningCacheUtxos) 02 https://github.com/bitcoin/bitcoin/pull/9274
3 2016-12-04 19:58:22 0|bitcoin-git|[13bitcoin] 15jonasschnelli pushed 3 new commits to 06master: 02 https://github.com/bitcoin/bitcoin/compare/2efcfa5acfac...4d955fc5824b
4 2016-12-04 19:58:23 0|bitcoin-git|13bitcoin/06master 14042f9fa 15Wladimir J. van der Laan: qt: Show progress overlay when clicking spinner icon...
5 2016-12-04 19:58:23 0|bitcoin-git|13bitcoin/06master 14827d9a3 15Wladimir J. van der Laan: qt: Replace NetworkToggleStatusBarControl with generic ClickableLabel...
6 2016-12-04 19:58:24 0|bitcoin-git|13bitcoin/06master 144d955fc 15Jonas Schnelli: Merge #9218: qt: Show progress overlay when clicking spinner icon...
7 2016-12-04 19:58:37 0|bitcoin-git|[13bitcoin] 15jonasschnelli closed pull request #9218: qt: Show progress overlay when clicking spinner icon (06master...062016_11_overlay_when_clicking_sync_icon) 02 https://github.com/bitcoin/bitcoin/pull/9218
8 2016-12-04 20:18:38 0|BlueMatt|jtimon: re: libconsensus: have you thought more about it after the discussions in milan? I'm still not a huge fan of exposing a ton of functions and prefer a simple refactor that a) makes it so that everything ProcessNewBlock calls isnt aware of disk positioning/etc and b) move all that stuff into a class which owns mapBlockIndex, chainActive, etc
9 2016-12-04 20:18:42 0|BlueMatt|jtimon: see https://github.com/TheBlueMatt/bitcoin/commits/2016-12-cconsensus
10 2016-12-04 20:20:20 0|jtimon|I keep thinking the same thing more or less
11 2016-12-04 20:21:26 0|jtimon|if you don't want to expose verifyHeader or verifyTx...I know some people want verifyHeader for SPV wallets, but not so sure about verifyTx
12 2016-12-04 20:21:49 0|jtimon|regarding process block, as discussed, I think we should expose verifyBlock instead
13 2016-12-04 20:22:43 0|BlueMatt|jtimon: why, though? if you expose ProcessNewBlock{,Headers} someone can implement a full spv client and full client all in the same codepaths, without exposing low-level partial-verification functions
14 2016-12-04 20:23:39 0|BlueMatt|(or, excuse me, without requiring users do their own utxo-management shit, while still giving them access to query the utxo state)
15 2016-12-04 20:23:40 0|jtimon|how can an SPV node calle ProcessNewBlock?
16 2016-12-04 20:23:48 0|BlueMatt|ProcessNewBlockHeaders
17 2016-12-04 20:23:51 0|jtimon|why would they want to depend on levelDB ?
18 2016-12-04 20:24:00 0|BlueMatt|nono, that part should be abstracted out
19 2016-12-04 20:24:01 0|BlueMatt|as discussed
20 2016-12-04 20:24:33 0|BlueMatt|eg see the blockstores in bitcoinj: https://github.com/bitcoinj/bitcoinj/tree/master/core/src/main/java/org/bitcoinj/store
21 2016-12-04 20:25:19 0|jtimon|oh, right, you were ok with abstracting storage, just wanted libconsensus to manage reorgs and all the rest in an abstracted way
22 2016-12-04 20:25:35 0|BlueMatt|yea, best to handle /all/ the complexity imo
23 2016-12-04 20:25:39 0|jtimon|isn't ProcessNewBlockHeaders an intermediate function?
24 2016-12-04 20:25:55 0|BlueMatt|no? it takes headers and adds them and selects the best headers chain it has seen
25 2016-12-04 20:27:06 0|jtimon|then I'm not sure what you mean by "intermediate" in this context
26 2016-12-04 20:27:57 0|BlueMatt|ehh, I apologize, I forgot that your definition of verifyBlock also means checking all the txes connect, ie essentially TestBlockValidity
27 2016-12-04 20:28:03 0|BlueMatt|instead of existing CheckBlock-type functions
28 2016-12-04 20:28:22 0|jtimon|well, no really testblockvelidity since that call connectBlock too
29 2016-12-04 20:28:42 0|BlueMatt|hmm?
30 2016-12-04 20:28:44 0|jtimon|not really
31 2016-12-04 20:28:59 0|BlueMatt|huh
32 2016-12-04 20:29:36 0|jtimon|my proposed verifyBlock just tells you whether a block is valid or not, but it doesn't do anything with it
33 2016-12-04 20:29:59 0|BlueMatt|indeed, like TestBlockValidity?
34 2016-12-04 20:31:56 0|jtimon|mhmm, connect block updates transactions, updates undo info and updates the view of the best block, no?
35 2016-12-04 20:32:13 0|BlueMatt|but TestBlockValidity throws away all that info afterwards
36 2016-12-04 20:32:21 0|jtimon|oh, I see
37 2016-12-04 20:32:26 0|BlueMatt|and you have to keep an updated-utxo-state as you connect the block to test validity
38 2016-12-04 20:32:43 0|jtimon|well, then yes, but without generating info than then throws away
39 2016-12-04 20:33:18 0|BlueMatt|you cand check a block without doing that
40 2016-12-04 20:33:24 0|jtimon|yeah, you need to update at least a view of the utxo
41 2016-12-04 20:33:50 0|jtimon|right, that was my idea for verifyBlock
42 2016-12-04 20:39:52 0|BlueMatt|jtimon: ok, so back to my original question...what do you think of not exposing that and actually having a utxo/disk-storage-abstraced full consensus layer
43 2016-12-04 20:39:53 0|BlueMatt|?
44 2016-12-04 20:40:56 0|jtimon|well, where I have te strongest opinion is in abstracting from storage
45 2016-12-04 20:41:11 0|jtimon|and agree on that
46 2016-12-04 20:41:12 0|BlueMatt|ok...?
47 2016-12-04 20:41:26 0|BlueMatt|oh, well sure, I dont think anyone wants libconsensus to require a certain storage layer
48 2016-12-04 20:41:56 0|jtimon|what would be the API? precessnewblock and processnewheaders?
49 2016-12-04 20:43:13 0|BlueMatt|jtimon: essentially, yes, PNB, PNBH, some initialization api (see https://github.com/TheBlueMatt/bitcoin/blob/2016-12-cconsensus/src/validation.cpp#L80) and a utxo-state-accessor/loadblock-accessor api
50 2016-12-04 20:43:18 0|jtimon|well, I do think some people want to depend on a concrete storage , but since it's neither of us, let's leave that aside
51 2016-12-04 20:44:02 0|jtimon|what about caches? any API for those?
52 2016-12-04 20:44:04 0|BlueMatt|well once the bitcoin core codebase "eats its own dogfood", as it were, we can take the storage layer we have and expose that optionally
53 2016-12-04 20:44:24 0|jtimon|right
54 2016-12-04 20:44:35 0|BlueMatt|no, caches are up to the client application, ie the client application gives libbitcoinconsensus pcoinsTip
55 2016-12-04 20:44:53 0|BlueMatt|similarly, in the future, maybe we can expose CCoinsViewCache or whatever
56 2016-12-04 20:45:00 0|BlueMatt|but for v1, its uneccessary
57 2016-12-04 20:45:19 0|jtimon|mhmm, I mean for example the versionbits cache
58 2016-12-04 20:45:46 0|jtimon|is that hidden to the caller or exposed somehow?
59 2016-12-04 20:46:08 0|BlueMatt|since its currently not exposed, hidden and present
60 2016-12-04 20:46:28 0|BlueMatt|dont expose anything unless its required, and for v1, dont worry too much about performance or memory usage
61 2016-12-04 20:47:04 0|sipa|the reason for not exposing would exactly be performance and memory usage, i think
62 2016-12-04 20:47:24 0|sipa|it's pretty hard to design a stable api that can be used efficiently for those things
63 2016-12-04 20:47:46 0|sipa|so i'd say make all state (caches, indexes, ...) by default part of the abstracted state
64 2016-12-04 20:48:01 0|sipa|and perhaps later introduce a way for the caller to install callbacks to override it, and provide their own
65 2016-12-04 20:48:05 0|jtimon|well, the api may change if consensus rules change
66 2016-12-04 20:52:48 0|jtimon|BlueMatt: I highly doubt some implementors (say libbitcoin) will use processNewBlock, then again I also doubted they were going to use verifyBlock directly either, I was hoping maybe verifyHeader, verifyTx + GetConsensusFlags
67 2016-12-04 20:53:40 0|BlueMatt|sipa: yea, except for v1 it doesnt matter, really - just dont expose anything and later, if we remove caches to make the library lighter, expose hooks to replace the caches
68 2016-12-04 20:53:56 0|bitcoin-git|[13bitcoin] 15morcos opened pull request #9276: Some minor testing cleanups (06master...06rpccleanup) 02 https://github.com/bitcoin/bitcoin/pull/9276
69 2016-12-04 20:53:58 0|jtimon|leaving that aside, I would like libconsensus to be the specification of the consensus rules for a given block to be valid, it seems that with your approach it would be more than that
70 2016-12-04 20:54:34 0|jtimon|but bitcoin core is currently way more than that, so...
71 2016-12-04 20:54:53 0|BlueMatt|jtimon: I mean the way any sane full node is structured there is an abstracted utxo/blocks-on-disk storage and something which checks everything and itnerfaces with that state
72 2016-12-04 20:55:08 0|jtimon|I also hope you don't intend to put half of the server package in the consensus package
73 2016-12-04 20:55:28 0|BlueMatt|jtimon: I see no reason why libbitcoinconsensus shouldnt repalce everything in between because if the goal is to reduce the risk of consensus incompatibility, we should replace everything we can
74 2016-12-04 20:55:52 0|BlueMatt|jtimon: I dont care what it weighs, only what it does....cutting down the weight is for later versions
75 2016-12-04 20:55:53 0|BlueMatt|:p
76 2016-12-04 20:57:55 0|jtimon|because some implementors (like libbitcoin) value more the ability to have their own concurrency model (ie without locks) than reducing risks of consensus incompatibility beyond verifyScript (which they also use only optionally)
77 2016-12-04 21:00:34 0|jtimon|I agree, that we can leave "cutting weight" for later (like we're leaving moving CFeeRate for later for very long), my point is that with your approach, even after you're done, it will be much bigger than "the specification for whether a block is valid or not"
78 2016-12-04 21:01:18 0|jtimon|it would be more like the specification of "how to follow and store the longest valid chain"
79 2016-12-04 21:01:54 0|jtimon|then someone will want prunning too...
80 2016-12-04 21:04:03 0|sipa|jtimon: yes, it's the specification for whether a blockchain is valid or not
81 2016-12-04 21:04:23 0|jtimon|sipa: processNewBlock does much more than that
82 2016-12-04 21:04:40 0|sipa|well, yes, due to necessity
83 2016-12-04 21:04:51 0|sipa|we can't track the validity of every single chain
84 2016-12-04 21:04:59 0|sipa|so we only have a utxo set at a tip
85 2016-12-04 21:05:47 0|jtimon|that's we, maybe other callers want to do things differently
86 2016-12-04 21:07:10 0|jtimon|then maybe someone else asks for "sharded prunning" or something else, and soon you end up again with a lot of things that don't have anything to do with consensus rules
87 2016-12-04 21:07:39 0|sipa|yes, so we can later provide ways to plug in your own state storage (utxo/caches/indexes/...)
88 2016-12-04 21:08:14 0|jtimon|sipa: oh, BlueMatt's approach already gives you an abstraction for the utxo and chain index storage
89 2016-12-04 21:08:50 0|sipa|oj
90 2016-12-04 21:08:52 0|sipa|ok
91 2016-12-04 21:09:58 0|jtimon|but my example is that if it supports prunning, that needs to be exposed somehow
92 2016-12-04 21:10:09 0|sipa|heh?
93 2016-12-04 21:10:20 0|sipa|how does block storage have anything to do with this
94 2016-12-04 21:10:37 0|jtimon|isn't prunning deleting blocks you are storing?
95 2016-12-04 21:10:50 0|sipa|yes, but blocks wouldn't be stored by libconsensus
96 2016-12-04 21:11:03 0|jtimon|they would with BlueMatt's approach
97 2016-12-04 21:11:15 0|sipa|hmm, that seems strange
98 2016-12-04 21:11:32 0|jtimon|if I undesrtood him correctly
99 2016-12-04 21:11:47 0|sipa|i would have an api where you plug in a way libconsensus to ask you for a block
100 2016-12-04 21:12:06 0|sipa|if yiu're abstracting utxo storage, i don't see why you wouldn't abstract nlock storage
101 2016-12-04 21:12:16 0|jtimon|that's why he needs something like https://github.com/bitcoinj/bitcoinj/blob/master/core/src/main/java/org/bitcoinj/store/BlockStore.java#L39
102 2016-12-04 21:12:18 0|sipa|which is even less consensus critical
103 2016-12-04 21:12:41 0|sipa|but that's the user that provides the blockstore
104 2016-12-04 21:12:45 0|sipa|not the library
105 2016-12-04 21:14:16 0|jtimon|right, the library provides an API and callbacks the implementor for stored data in both cases, but in bluematt's case, it also updates the storage, it doesn't use it in a read only way
106 2016-12-04 21:14:41 0|jtimon|maybe prunning was a bad example, I agree you don't need to put it in libconsensus even in that case
107 2016-12-04 21:16:00 0|gmaxwell|I think abstracting things like all this storage maybe getting into the realm of imaginary users... is there really many users that want to make no change (not even instrumentation) to consensus but is very interested at implementing their own UTXO database?
108 2016-12-04 21:16:00 0|jtimon|in my case, it takes a block and says valid or validation error x and does nothing more than that
109 2016-12-04 21:16:32 0|sipa|jtimon: but you need to reimplement a lot of complicated logic in your application for it
110 2016-12-04 21:16:37 0|sipa|utxo caching is hard
111 2016-12-04 21:16:48 0|sipa|block index tracking with skiplists is not trivial
112 2016-12-04 21:16:55 0|jtimon|gmaxwell: I agree that part of the problem here is not having a clear idea of the users or their preferences
113 2016-12-04 21:17:00 0|gmaxwell|I thought the goal for libconsensus is that so that when someone wants to make their own custom wallet thing they wouldn't need to reimplment any of the consensus logic, just to create their own application logic.
114 2016-12-04 21:17:20 0|jtimon|gmaxwell: but it seems that your objection would apply to both my approach and matt's
115 2016-12-04 21:17:26 0|sipa|i really don't care about users at this point - we don't have any, and have no easy way to build somwthing others could use
116 2016-12-04 21:17:40 0|sipa|i care about separating consensus from nonconsensus logic
117 2016-12-04 21:18:00 0|gmaxwell|if so that would really mean all the chain management would really be inside the library.
118 2016-12-04 21:18:19 0|gmaxwell|sipa: okay so for the purpose of consensus safty of changes, the caches and utxo database are all consensus critical--
119 2016-12-04 21:18:35 0|sipa|gmaxwell: fair enough, but so is the c++ standard library
120 2016-12-04 21:18:42 0|jtimon|gmaxwell: right, matt puts the chain management inside, but also provides an abstraction for storage
121 2016-12-04 21:19:20 0|sipa|i think having block storage outside the library is perfectly fine, as consensus logic really does not need blocks except for reorgs
122 2016-12-04 21:19:30 0|gmaxwell|jtimon: Providing an abstraction for IO though is different than something that expects the user to implement a complex database.
123 2016-12-04 21:20:01 0|sipa|even utxo storage i can be convinced of, i think
124 2016-12-04 21:20:22 0|sipa|but block chain tracking (cblockindex/mapblockindex/...) should really be inside
125 2016-12-04 21:20:38 0|sipa|it's highly dependent on how we manage chains
126 2016-12-04 21:20:55 0|jtimon|providing a wrapper that includes the same storage as bitcoin core, as an extended library should be easy
127 2016-12-04 21:21:21 0|gmaxwell|so long as the requirements are very clear and simple... where no bitcoin specific understanding is required... sure. though doing that without breaking efficiency might be hard.
128 2016-12-04 21:21:29 0|sipa|i don't want every cblockindex access to go through wrapper calls
129 2016-12-04 21:22:01 0|gmaxwell|e.g. there are ordering requirements for fixation of block data vs utxo data to disk.
130 2016-12-04 21:22:02 0|jtimon|yep, efficiency is the main concern with my approach I think
131 2016-12-04 21:23:39 0|jtimon|btw, as always comments on https://github.com/bitcoin/bitcoin/pull/8493 welcomed
132 2016-12-04 21:25:10 0|jtimon|this kind of thing (for the abstracted storage) is disruptive though https://github.com/bitcoin/bitcoin/pull/8493/commits/0a1ff996c5db5265742e3e90f3690a0b8d9cf045
133 2016-12-04 21:26:51 0|jtimon|and the way I implemented it there, is also inneficient for bitcoin core
134 2016-12-04 21:36:36 0|jtimon|anyway, there's 2 separated topics here
135 2016-12-04 21:36:47 0|jtimon|whether to abstract and expose storage
136 2016-12-04 21:37:22 0|jtimon|and whether or not to manage reorgs and other things beyond "check whether a given block is valid or not"
137 2016-12-04 21:39:42 0|jtimon|my answers are yes and no, matt's is yes and yes. If gmaxwell's is no and yes, and sipa's are yes and yes, we have all the options represented :p
138 2016-12-04 21:43:02 0|jtimon|s/sipa's are no and no/
139 2016-12-04 21:45:36 0|sipa|the first thing we should probably do is separate CBlockIndex into block storage data (file, offset, undo, ...), and chain data (block index, work, validation)
140 2016-12-04 21:46:02 0|sipa|that would be required if we'd want to abstract block storage out from a library
141 2016-12-04 21:46:09 0|sipa|but i thibk it shoukd hapoen regardless
142 2016-12-04 21:46:32 0|jtimon|I don't need to do that in my approach
143 2016-12-04 21:46:35 0|sipa|the block storage data doesn't even need to be in memory permnantly
144 2016-12-04 21:46:51 0|sipa|i understand
145 2016-12-04 21:47:08 0|sipa|but my point is that we should do that, even if there never is a library at all
146 2016-12-04 21:48:32 0|jtimon|chain.o and coins.o are already separated, I'm not entirely sure I understand what you mean
147 2016-12-04 21:50:34 0|sipa|CBlockIndex stores things about where blocks are on disk
148 2016-12-04 21:50:44 0|jtimon|oh, I see
149 2016-12-04 21:50:44 0|sipa|(which has nothing to do with consensus)
150 2016-12-04 21:50:57 0|sipa|and things like validation flags and chainwork
151 2016-12-04 21:51:01 0|sipa|and the block header
152 2016-12-04 21:51:04 0|sipa|which do
153 2016-12-04 21:53:45 0|jtimon|regarding the validation flags, I replaced #7779 with https://github.com/bitcoin/bitcoin/pull/9271 (although got errors in unittests because we're already relying on the consensus flags being coupled for testing it seems)
154 2016-12-04 21:53:47 0|gribble|https://github.com/bitcoin/bitcoin/issues/7779 | An error has occurred and has been logged. Please contact this bot's administrator for more information.
155 2016-12-04 21:56:24 0|jtimon|see https://github.com/bitcoin/bitcoin/pull/9271/commits/7a90173b839e682932426f7d6ac91d2df88e2cfb
156 2016-12-04 22:04:53 0|bitcoin-git|[13bitcoin] 15s-matthew-english opened pull request #9277: clear typo "skipepd" changed to "skipped" (06master...06patch-12) 02 https://github.com/bitcoin/bitcoin/pull/9277
157 2016-12-04 22:11:50 0|bitcoin-git|[13bitcoin] 15MarcoFalke closed pull request #9277: clear typo "skipepd" changed to "skipped" (06master...06patch-12) 02 https://github.com/bitcoin/bitcoin/pull/9277
158 2016-12-04 22:56:36 0|sdaftuar|gmaxwell: regarding bumpfee... i'd been advising mrbandrews to try to simplify what the rpc command is doing as much as possible, with the hope of getting a helpful utility in place tht we could layer more advanced behavior on top of
159 2016-12-04 22:57:05 0|sdaftuar|gmaxwell: so in particular, i wasn't aware of a robust way to identify the change output, so i suggested for this rpc to have the user supply it
160 2016-12-04 22:57:14 0|sdaftuar|is there a robust way to identify the change output?
161 2016-12-04 22:58:11 0|sipa|yes, IsMine() and not in address book
162 2016-12-04 23:03:21 0|sdaftuar|sorry i'm so unfamiliar with this... i guess there's an IsChange() function, with a TODO saying that this might not work with multisig behavior in the future. is that not a concern now?
163 2016-12-04 23:09:11 0|gmaxwell|sdaftuar: We really should be storing the data in the wallet, but just using IsMine would be better than asking for it.
164 2016-12-04 23:09:42 0|gmaxwell|(I think specifying it is basically something that would only be useful to the same people who could use createrawtransaction)
165 2016-12-04 23:10:25 0|gmaxwell|Simplifying it is good, but we can't make it a hazard to users-- which is why I was saying it needed to somehow assure that the outputs will not get spent while unconfirmed.
166 2016-12-04 23:11:14 0|gmaxwell|Simiarly, just diminishing the change and never adding inputs will produce transactions which won't relay (dust). though I suppose instead of being able to add inputs it could just limit itself to places where that wouldn't be the case.
167 2016-12-04 23:11:44 0|sdaftuar|gmaxwell: i haven't looked at the PR lately but i thought it was going to do the right thing with dust.
168 2016-12-04 23:11:54 0|gmaxwell|Because of the need to restrict dependant transaction chains I thought a 'bump everything' might be simpler.
169 2016-12-04 23:12:20 0|gmaxwell|sdaftuar: it didn't look like it could add inputs to me but equally possible that I was confused.
170 2016-12-04 23:13:02 0|sdaftuar|gmaxwell: oh it won't currently add inputs, as i recall, but i think if it would reduce the change output to less than dust, then it would get dropped altogether
171 2016-12-04 23:13:14 0|sdaftuar|(that's what i recall some discussion of, anyway)
172 2016-12-04 23:13:34 0|sdaftuar|are you saying though that you don't think it should allow feebumping a tx that has descendants?
173 2016-12-04 23:13:55 0|sdaftuar|i thought that was part of the point of this RPC, to get that right.
174 2016-12-04 23:18:25 0|gmaxwell|imagine you have txn A, and B that spends A. Then you bump A creating a replaceme A' that replaces A/B. Now you make nother spend that spends the output of A' ... you really need to draft two versions of it one that spends B and one that spends A'... and switch to relaying the other depending on what confirms.
175 2016-12-04 23:18:39 0|gmaxwell|I thought that was too advanced, which is why I was suggesting prohibiting descendants.
176 2016-12-04 23:18:49 0|sdaftuar|ahh right
177 2016-12-04 23:18:58 0|sdaftuar|wow, i totally missed that
178 2016-12-04 23:19:51 0|gmaxwell|GAit has implemented bumping in the green address wallet and I think it deals with all this stuff.
179 2016-12-04 23:20:01 0|sdaftuar|PRs welcome? :)
180 2016-12-04 23:20:40 0|gmaxwell|even my suggestion of making unspendable for one confirm is a bit risky, since it couple be reorged out. :(
181 2016-12-04 23:20:44 0|sdaftuar|so i think bumpunconfirmed is a good idea, but i worry about old transactions that have long since been double spent
182 2016-12-04 23:20:53 0|gmaxwell|esp since there is non-trivial amounts of hashpower that won't take replacements.
183 2016-12-04 23:20:55 0|sdaftuar|or rather, distinguishing those from other unconfirmed transactions
184 2016-12-04 23:21:07 0|gmaxwell|well we track conflicted transactions.
185 2016-12-04 23:21:11 0|sdaftuar|but not perfectly
186 2016-12-04 23:21:34 0|sdaftuar|we can't track conflicted transactions where the conflict chain is broken with tx's not in our wallet
187 2016-12-04 23:22:34 0|gmaxwell|Point. Though it would be sufficient for this functionality to only work on transactions where the closure of unconfirmed parents are all in the wallet, I guess.
188 2016-12-04 23:25:38 0|gmaxwell|I wonder how often someone has multiple unconfirmeds pending and doesn't want to just bump them all. Usually it will save them total fees if they do so.
189 2016-12-04 23:25:54 0|gmaxwell|ignoring crazy corner cases like some being doublespent.
190 2016-12-04 23:26:58 0|sdaftuar|yeah i think it does seem like a useful feature
191 2016-12-04 23:27:40 0|gmaxwell|it just seemed simpler to me than getting all the corner cases right.
192 2016-12-04 23:28:14 0|sdaftuar|well i think that has a bunch of corner cases too. you probably want to make sure you conflict with anything that might possibly confirm, so you have to include even abandoned transactions right?
193 2016-12-04 23:28:27 0|sdaftuar|(maybe even 1-confirm tx's!)
194 2016-12-04 23:28:41 0|sdaftuar|er 1-conflicted
195 2016-12-04 23:29:12 0|gmaxwell|You need to conflict with anything whos outputs you include, so that you don't potentially double-pay.
196 2016-12-04 23:30:07 0|sdaftuar|i guess that raises a good UI point
197 2016-12-04 23:30:31 0|sdaftuar|probably the user would want to see all the outputs and transactions being bumped, and ack that?
198 2016-12-04 23:30:48 0|sdaftuar|in case of the really old wallet tx that the user forgot about problem
199 2016-12-04 23:32:00 0|sdaftuar|i dunno, i guess i just assume that heavy wallet users that have been running for a while must have lots of those, maybe that's not a good assumption
200 2016-12-04 23:32:06 0|sdaftuar|?
201 2016-12-04 23:32:57 0|gmaxwell|I think it would work like this: take the set of unconfirmed transactions which are AllFromMe (whatever the test that excludes coinjoins is called). Then eliminate all transactions whos parents aren't either all AllFromMe or 6 confirmed. Then gather up all their outputs, eliminate all the IsMine outputs, and generate a new transaction which conflicts each transaction in the set, and pays suffi
202 2016-12-04 23:33:03 0|gmaxwell|ciently more fees. Then its change output (if any) should be marked so that it will not be treated as IsFromMe for spending purposes. (UI wise we should encourage somewhat overpaying due to this last point)
203 2016-12-04 23:34:01 0|gmaxwell|I don't think most users have a lot of long lost transactions. I hope not. :) since they'll pretty likely to lose coins if they ditch the wallet thinking the balance was zero.
204 2016-12-04 23:34:06 0|sdaftuar|that seems like it ought to work.
205 2016-12-04 23:34:18 0|sdaftuar|do we have a way right now to do the mark-change-as-unspendable thing?
206 2016-12-04 23:34:38 0|gmaxwell|We have something that doesn't do what we want here.
207 2016-12-04 23:34:47 0|gmaxwell|(lockunspent)
208 2016-12-04 23:35:35 0|gmaxwell|For this, I think we need to add a boolen to the wallet txn and have isfromme check that.
209 2016-12-04 23:36:07 0|gmaxwell|Later we could have more advanced bumping logic that does the make-multiple-transaction-versions.
210 2016-12-04 23:37:57 0|gmaxwell|sdaftuar: if you really think that there are many users with txn stuck never confirmed... then we really need to add a 'Pending Payments' -- sum of confirmed outputs which are spent by unconfirmed transactions -- in the UI/rpc. So that people don't discard wallets as empty when they're really not.
211 2016-12-04 23:38:27 0|sdaftuar|gmaxwell: actually, maybe i am missing something. with your proposed algorithm, what would happen if you bumpunconfirmed twice?
212 2016-12-04 23:39:24 0|gmaxwell|You would ignore the existing bump (it would fail the AllFrom me in step 1) and make a new bump of all the other unconfirmed transactions in your wallet.
213 2016-12-04 23:40:16 0|sdaftuar|i didn't follow why it would fail the AllFromMe step, but if you managed to construct a new tx that didn't conflict with the older bumped fee one, you're screwed right?
214 2016-12-04 23:41:26 0|gmaxwell|It would fail the all from me, because I propose the change output of the bumb be flagged specifically so that it does. ... so that under no condition wall the wallet spend it until it is 6 confirmed. Specfically to avoid having to deal with things spending it then having the originals confirm.
215 2016-12-04 23:42:06 0|sdaftuar|isn't IsAllFromMe purely a function of the tx inputs?
216 2016-12-04 23:42:27 0|gmaxwell|The new tx doesn't need to conflict with it, so long as it doesn't pay to any of its outputs, which it won't because it wasn't included in the set of available txn.
217 2016-12-04 23:43:03 0|sdaftuar|i think i am pretty confused right now, why don't i spend another day thinking about the wallet and then rejoin this conversation
218 2016-12-04 23:43:34 0|gmaxwell|It's a function of the input's outputs. Is every input an output of mine.
219 2016-12-04 23:44:23 0|gmaxwell|sdaftuar: this stuff makes my head hurt too. I really want to get some kind of bumping in ASAP, it's hard to come up with a minimal feature...
220 2016-12-04 23:45:50 0|sdaftuar|one last try before i call it a night -- let's say i have tx1, tx2, tx3 -- each one is IsAllFromMe. i bump and create tx4, which conflicts with each, and pays all their outputs, and the change output of tx4 is specially flagged or whatever to be unspendable until it confirms.
221 2016-12-04 23:45:59 0|sdaftuar|wont't tx4 still be IsAllFromMe?
222 2016-12-04 23:47:00 0|sdaftuar|(actually i have to run, back later)
223 2016-12-04 23:47:02 0|gmaxwell|Right, okay -- I should have said IsMine and IsAllFromMe -- the flagging should make it not IsMine.
224 2016-12-04 23:47:06 0|gmaxwell|sdaftuar: ttyl
225 2016-12-04 23:58:45 0|BlueMatt|gmaxwell: ok, I missed a bunch of scrollback, but hell yes...everyone wants the utxo db in their own format so that they can do other queries against it
226 2016-12-04 23:58:59 0|BlueMatt|jtimon: no, I absolutely think we should abstract out block storage
227 2016-12-04 23:59:17 0|BlueMatt|jtimon: hell, the branch I sent you did that for ConnectBlock/DisconnectBlock in the first commit
228 2016-12-04 23:59:46 0|jtimon|right, I said "yes and yes" for you