1 2018-05-10 02:47:39 0|glaksmono|anyone is around?
2 2018-05-10 02:50:15 0|sipa|glaksmono: sure
3 2018-05-10 04:29:17 0|bitcoin-git|[13bitcoin] 15leiyong1413 opened pull request #13206: Zerotoone (06master...06zerotoone) 02https://github.com/bitcoin/bitcoin/pull/13206
4 2018-05-10 04:30:02 0|bitcoin-git|[13bitcoin] 15fanquake closed pull request #13206: Zerotoone (06master...06zerotoone) 02https://github.com/bitcoin/bitcoin/pull/13206
5 2018-05-10 06:21:38 0|kallewoof|promag: Do you agree with the conclusions Nicolas and I made in #12634 regarding 0/1?
6 2018-05-10 06:21:40 0|gribble|https://github.com/bitcoin/bitcoin/issues/12634 | [refactor] Make TransactionWithinChainLimit more flexible by kallewoof ÷ Pull Request #12634 ÷ bitcoin/bitcoin ÷ GitHub
7 2018-05-10 06:58:13 0|jonasschnelli|What exactly does the undocumented -forcecompactdb do?
8 2018-05-10 06:59:00 0|jonasschnelli|or say, what is the effect of leveldb's pdb->CompactRange(&slKey1, &slKey2);?
9 2018-05-10 07:08:03 0|fanquake|jonasschnelli #10526
10 2018-05-10 07:08:07 0|gribble|https://github.com/bitcoin/bitcoin/issues/10526 | Force on-the-fly compaction during pertxout upgrade by sipa ÷ Pull Request #10526 ÷ bitcoin/bitcoin ÷ GitHub
11 2018-05-10 07:09:09 0|fanquake|& #10985
12 2018-05-10 07:09:11 0|gribble|https://github.com/bitcoin/bitcoin/issues/10985 | Add undocumented -forcecompactdb to force LevelDB compactions by sipa ÷ Pull Request #10985 ÷ bitcoin/bitcoin ÷ GitHub
13 2018-05-10 07:09:11 0|jonasschnelli|fanquake: Thanks for the research!
14 2018-05-10 07:09:11 0|kallewoof|jonasschnelli: The docs seem to indicate it clears up some space: https://godoc.org/github.com/syndtr/goleveldb/leveldb#DB.CompactRange
15 2018-05-10 07:09:23 0|jonasschnelli|kallewoof: indeed. Thanks
16 2018-05-10 08:27:18 0|jonasschnelli|When I execute "generate 10" on a regtest node, it will only inv the most recent block to the connected peer
17 2018-05-10 08:27:19 0|jonasschnelli|https://pastebin.com/raw/m7b39bRX
18 2018-05-10 08:27:24 0|jonasschnelli|Is this also expected on mainnet?
19 2018-05-10 08:27:32 0|jonasschnelli|Why only the last block? Concurrency issue?
20 2018-05-10 08:41:06 0|promag|kallewoof: I'll take a look and I'll get back to you later ok?
21 2018-05-10 08:52:39 0|kallewoof|promag: No rush!
22 2018-05-10 11:22:19 0|bitcoin-git|[13bitcoin] 15kallewoof opened pull request #13208: wallet rpc: constraints in send* methods (06master...06feature-sendtoaddress-constraints) 02https://github.com/bitcoin/bitcoin/pull/13208
23 2018-05-10 11:30:54 0|fanquake|kallewoof looks like Travis is failing early on the CHECK DOC
24 2018-05-10 12:27:03 0|bitcoin-git|[13bitcoin] 15glaksmono opened pull request #13209: [WIP] Refactoring platform-specific code in util.h/util.cpp (06master...06bitcoin-12697) 02https://github.com/bitcoin/bitcoin/pull/13209
25 2018-05-10 14:33:09 0|bitcoin-git|13bitcoin/06master 1409c6699 15Suhas Daftuar: [qa] Handle disconnect_node race...
26 2018-05-10 14:33:09 0|bitcoin-git|[13bitcoin] 15MarcoFalke pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/196c5a947a07...f3e747ee775f
27 2018-05-10 14:33:10 0|bitcoin-git|13bitcoin/06master 14f3e747e 15MarcoFalke: Merge #13201: [qa] Handle disconnect_node race...
28 2018-05-10 14:34:07 0|bitcoin-git|[13bitcoin] 15MarcoFalke closed pull request #13201: [qa] Handle disconnect_node race (06master...062018-05-rpc-disconnect-race) 02https://github.com/bitcoin/bitcoin/pull/13201
29 2018-05-10 14:51:06 0|promag|achow101: hi, you have a bunch of nits in #13112: `try {` instead of `try\n` and missing space in `fprintf(stderr,"Error...`
30 2018-05-10 14:51:09 0|gribble|https://github.com/bitcoin/bitcoin/issues/13112 | Throw an error for unknown args by achow101 ÷ Pull Request #13112 ÷ bitcoin/bitcoin ÷ GitHub
31 2018-05-10 14:54:24 0|Lauda|Is gettxoutsetinfo supposed to take a long time to process?
32 2018-05-10 15:06:02 0|wumpus|Lauda: yes
33 2018-05-10 15:06:13 0|wumpus|Lauda: should be in the help message
34 2018-05-10 15:06:35 0|Lauda|Thanks!
35 2018-05-10 15:06:56 0|bitcoin-git|13bitcoin/06master 1412d1b77 15lmanners: [tests] Fixed intermittent failure in p2p_sendheaders.py....
36 2018-05-10 15:06:56 0|bitcoin-git|[13bitcoin] 15MarcoFalke pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/f3e747ee775f...1c582503507b
37 2018-05-10 15:06:57 0|bitcoin-git|13bitcoin/06master 141c58250 15MarcoFalke: Merge #13192: [tests] Fixed intermittent failure in p2p_sendheaders.py....
38 2018-05-10 15:07:46 0|bitcoin-git|[13bitcoin] 15MarcoFalke closed pull request #13192: [tests] Fixed intermittent failure in p2p_sendheaders.py. (06master...06p2p_sendheaders) 02https://github.com/bitcoin/bitcoin/pull/13192
39 2018-05-10 15:31:24 0|bitcoin-git|[13bitcoin] 15jbampton opened pull request #13210: Trivial: Remove trailing whitespace from Python files (06master...06remove-whitespace) 02https://github.com/bitcoin/bitcoin/pull/13210
40 2018-05-10 17:28:20 0|bitcoin-git|[13bitcoin] 15laanwj opened pull request #13211: Use a semaphore or pipe for shutdown notification (06master...062018_05_shutdown_notification) 02https://github.com/bitcoin/bitcoin/pull/13211
41 2018-05-10 17:28:32 0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #13186: Use a semaphore to trigger shutdown procedures (06master...06shutdown-cv) 02https://github.com/bitcoin/bitcoin/pull/13186
42 2018-05-10 17:49:47 0|bitcoin-git|[13bitcoin] 15lmanners opened pull request #13212: Net: Fixed a race condition when disabling the network. (06master...06setnetworkactive) 02https://github.com/bitcoin/bitcoin/pull/13212
43 2018-05-10 18:53:48 0|Odin|anybody has a cfw for antminer s5 or s7?
44 2018-05-10 18:55:39 0|wumpus|no (but this is probably not the right channel to ask)
45 2018-05-10 18:55:55 0|MarcoFalke|Meeting in 3 min?
46 2018-05-10 18:57:07 0|wumpus|yes
47 2018-05-10 18:57:15 0|MarcoFalke|Proposed topic: Cache witness hash in CTransaction #13011
48 2018-05-10 18:57:17 0|gribble|https://github.com/bitcoin/bitcoin/issues/13011 | Cache witness hash in CTransaction by MarcoFalke ÷ Pull Request #13011 ÷ bitcoin/bitcoin ÷ GitHub
49 2018-05-10 18:59:46 0|wumpus|#startmeeting
50 2018-05-10 18:59:47 0|lightningbot|Meeting started Thu May 10 19:00:27 2018 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
51 2018-05-10 18:59:47 0|lightningbot|Useful Commands: #action #agreed #help #info #idea #link #topic.
52 2018-05-10 18:59:54 0|promag|hi
53 2018-05-10 18:59:58 0|jonasschnelli|hi
54 2018-05-10 19:00:00 0|wumpus|#bitcoin-core-dev Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr btcdrak sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier jl2012 achow101 meshcollider jnewbery maaku fanquake promag provoostenator
55 2018-05-10 19:00:09 0|provoostenator|Hi
56 2018-05-10 19:00:10 0|cfields|hi
57 2018-05-10 19:00:18 0|wumpus|proposed topics?
58 2018-05-10 19:00:23 0|sipa|ohai
59 2018-05-10 19:00:30 0|jimpo|hi
60 2018-05-10 19:00:34 0|jcorgan|hey folks
61 2018-05-10 19:00:55 0|wumpus|hello
62 2018-05-10 19:00:57 0|jimpo|topic proposal: review coordination
63 2018-05-10 19:01:38 0|wumpus|ok thanks,let's start with the usual
64 2018-05-10 19:01:39 0|wumpus|#topic High priority for review
65 2018-05-10 19:01:47 0|wumpus|https://github.com/bitcoin/bitcoin/projects/8
66 2018-05-10 19:02:19 0|achow101|hi
67 2018-05-10 19:02:22 0|wumpus|#10740 #13097 #12979 #12560 #10757
68 2018-05-10 19:02:28 0|gribble|https://github.com/bitcoin/bitcoin/issues/10740 | [wallet] `loadwallet` RPC - load wallet at runtime by jnewbery ÷ Pull Request #10740 ÷ bitcoin/bitcoin ÷ GitHub
69 2018-05-10 19:02:29 0|gribble|https://github.com/bitcoin/bitcoin/issues/13097 | ui: Support wallets loaded dynamically by promag ÷ Pull Request #13097 ÷ bitcoin/bitcoin ÷ GitHub
70 2018-05-10 19:02:33 0|gribble|https://github.com/bitcoin/bitcoin/issues/12979 | Split validationinterface into parallel validation/mempool interfaces by TheBlueMatt ÷ Pull Request #12979 ÷ bitcoin/bitcoin ÷ GitHub
71 2018-05-10 19:02:38 0|gribble|https://github.com/bitcoin/bitcoin/issues/10757 | RPC: Introduce getblockstats to plot things by jtimon ÷ Pull Request #10757 ÷ bitcoin/bitcoin ÷ GitHub
72 2018-05-10 19:02:43 0|gribble|https://github.com/bitcoin/bitcoin/issues/12560 | [wallet] Upgrade path for non-HD wallets to HD by achow101 ÷ Pull Request #12560 ÷ bitcoin/bitcoin ÷ GitHub
73 2018-05-10 19:03:04 0|jamesob|hi
74 2018-05-10 19:03:09 0|morcos|i'm back
75 2018-05-10 19:03:15 0|wumpus|welcome back!
76 2018-05-10 19:03:19 0|sipa|hi back, i'm pieter
77 2018-05-10 19:03:28 0|morcos|oh man...
78 2018-05-10 19:03:31 0|luke-jr|XD
79 2018-05-10 19:03:38 0|BlueMatt|mr back?
80 2018-05-10 19:04:15 0|wumpus|10740 should be pretty close to mergeable (but getting unicorns right now)
81 2018-05-10 19:04:37 0|jimpo|Can I request #12254 for hi pri?
82 2018-05-10 19:04:41 0|gribble|https://github.com/bitcoin/bitcoin/issues/12254 | BIP 158: Compact Block Filters for Light Clients by jimpo ÷ Pull Request #12254 ÷ bitcoin/bitcoin ÷ GitHub
83 2018-05-10 19:05:13 0|BlueMatt|god if this unicorns thing persists we're gonna have to move off github
84 2018-05-10 19:05:35 0|BlueMatt|(actually)
85 2018-05-10 19:05:52 0|wumpus|jimpo: ye
86 2018-05-10 19:06:11 0|jimpo|The GitHub load timeouts seem to be location specific maybe -- have you tried loading over VPN from different places?
87 2018-05-10 19:06:17 0|jimpo|Like VPN near GitHub servers?
88 2018-05-10 19:06:23 0|promag|yeah imo 10740 is good to go
89 2018-05-10 19:06:38 0|BlueMatt|mostly it works if you refresh enough or are logged out, but neither of those is a solution when the refresh count is about 10
90 2018-05-10 19:06:43 0|wumpus|no, I haven't tried using a vpn, when I tried tor it didn't help though
91 2018-05-10 19:06:53 0|BlueMatt|but we cant use a platform where half the contributors cant load prs
92 2018-05-10 19:07:02 0|jimpo|I left some late comments on 10740 yesterday. Biggest one is around the memory leak fix.
93 2018-05-10 19:07:21 0|wumpus|usually refreshing helps but sometimes it doesn't for very large PRs with lots of comments
94 2018-05-10 19:07:32 0|wumpus|agree github is pretty much useless this way
95 2018-05-10 19:07:44 0|promag|jimpo: +1, beside existing comments
96 2018-05-10 19:07:50 0|jamesob|everyone knows the emulate-mobile/incognito workaround, right?
97 2018-05-10 19:08:52 0|jamesob|(if you don't: use chrome developer tools to emulate a mobile device and problematic PRs should load)
98 2018-05-10 19:09:19 0|wumpus|jamesob: thanks
99 2018-05-10 19:10:10 0|wumpus|#topic Cache witness hash in CTransaction #13011 (MarcoFalke)
100 2018-05-10 19:10:11 0|gribble|https://github.com/bitcoin/bitcoin/issues/13011 | Cache witness hash in CTransaction by MarcoFalke ÷ Pull Request #13011 ÷ bitcoin/bitcoin ÷ GitHub
101 2018-05-10 19:10:20 0|MarcoFalke|Background: The witness hash is used for all loose transactions, so caching it would speed up validation (e.g. ATMP and compact block relay). Also, the witness hash is equal to the "normal hash" for transactions without a witness, so there is overhead for rescan/reindex is currently minimal (since there are not many transactions with witness). The gains of caching the witness hash dwarf any overhead during rescan/reindex, imo. And of course, we
102 2018-05-10 19:10:21 0|MarcoFalke|can just rework rescan in a future pr.
103 2018-05-10 19:10:58 0|MarcoFalke|The code changes are trivial, so at least that shouldn't be an issue
104 2018-05-10 19:11:29 0|jamesob|would be nice if that paragraph was in the PR description
105 2018-05-10 19:11:49 0|sipa|Downside is that it makes the transactions larger, and hardcodes some validation sprcific logic into the transaction data structure (which for example also affects serving blocks from disks etc)
106 2018-05-10 19:12:12 0|BlueMatt|upside is we rectify a significant performance regression
107 2018-05-10 19:12:14 0|sipa|So my view is that we should separate the transactions-for-validation tyoe and simple mutable transactions
108 2018-05-10 19:12:27 0|BlueMatt|and obviously all pre-segwit-activation blocks will not be served any slower
109 2018-05-10 19:12:29 0|MarcoFalke|sipa: I think those can easily be fixed in future prs (I have one open for the blocks serving from disks, and wumpus as well)
110 2018-05-10 19:12:45 0|sipa|MarcoFalke: i'm aware, not objecting to amything
111 2018-05-10 19:12:47 0|BlueMatt|personally, I find it really unacceptable that we have this huge performance regression and arent taking it as something to be fixed asap
112 2018-05-10 19:12:59 0|MarcoFalke|sipa: I know, just posting for reference
113 2018-05-10 19:13:00 0|sipa|just pointing out that we don't want to do this work everywhere
114 2018-05-10 19:13:14 0|sipa|so concept ack, if we commit to separating those types
115 2018-05-10 19:13:27 0|wumpus|agree with sipa
116 2018-05-10 19:13:31 0|BlueMatt|yes, imo we should merge the per as-is now, and then when we look towards MarcoFalke's next pr splitting out the types (which is gonna be a lot more work) we will probably pull out both hashes then anyway
117 2018-05-10 19:13:42 0|sipa|there are other reasons for separating those types as well, btw
118 2018-05-10 19:13:54 0|MarcoFalke|For reference, the "separating those types" is hidden in #13098
119 2018-05-10 19:13:55 0|wumpus|let's not make the code a mess to rush ahead
120 2018-05-10 19:13:57 0|gribble|https://github.com/bitcoin/bitcoin/issues/13098 | Skip tx-rehashing on historic blocks by MarcoFalke ÷ Pull Request #13098 ÷ bitcoin/bitcoin ÷ GitHub
121 2018-05-10 19:13:59 0|BlueMatt|yes, I mean just fixing the txid hashing would be hugely nice
122 2018-05-10 19:14:00 0|sipa|such as using more efficient memory representation of scripts without individial mallocs etc
123 2018-05-10 19:14:11 0|kanzure|hi.
124 2018-05-10 19:14:46 0|wumpus|yes
125 2018-05-10 19:14:55 0|cfields|topic suggestion: ^^ one big malloc
126 2018-05-10 19:15:02 0|cfields|(when this one's done)
127 2018-05-10 19:15:12 0|BlueMatt|wumpus: if anything it makes the code simpler cause the witness and non-witness code is mirrored
128 2018-05-10 19:15:29 0|MarcoFalke|Should I put #13011 in hi priority and for backport?
129 2018-05-10 19:15:31 0|gribble|https://github.com/bitcoin/bitcoin/issues/13011 | Cache witness hash in CTransaction by MarcoFalke ÷ Pull Request #13011 ÷ bitcoin/bitcoin ÷ GitHub
130 2018-05-10 19:15:39 0|wumpus|BlueMatt: I just don't like confounding all kinds of aspects on the CTransaction object through, it's used in too many places
131 2018-05-10 19:15:55 0|BlueMatt|good thing miners dont pay much attention, or they wouldnt be mining segwit txn
132 2018-05-10 19:16:40 0|BlueMatt|wumpus: agreed, but point being the txid and wtxid code being symmetric isnt that much of a "mess"
133 2018-05-10 19:16:51 0|wumpus|BlueMatt: right...
134 2018-05-10 19:16:56 0|BlueMatt|and obviously agree we should be looking into rescan with a non-txid/non-wtxid type
135 2018-05-10 19:17:03 0|sipa|fait
136 2018-05-10 19:17:06 0|sipa|fair
137 2018-05-10 19:17:10 0|BlueMatt|but I have a feeling that is gonna be a lot of work
138 2018-05-10 19:17:19 0|BlueMatt|or at least take a while review-wise
139 2018-05-10 19:18:28 0|wumpus|MarcoFalke: ok added to high priority
140 2018-05-10 19:18:34 0|MarcoFalke|wumpus: thx
141 2018-05-10 19:18:46 0|MarcoFalke|I will update the OP, as requested by jamesob
142 2018-05-10 19:18:53 0|wumpus|not sure about the backport, I think it makes sense to focus on master for performance optimizations
143 2018-05-10 19:19:18 0|MarcoFalke|I don't care about the backport, but BlueMatt was asking for it
144 2018-05-10 19:19:31 0|BlueMatt|it slows down compact block relay, too, which is mostly why it sucks
145 2018-05-10 19:19:32 0|wumpus|unless it's really easy and low risk to backport it
146 2018-05-10 19:19:37 0|BlueMatt|otherwise I might care less
147 2018-05-10 19:19:43 0|cfields|agree with wumpus. I'm sure the changes are straightforward, but that's still risky for backport
148 2018-05-10 19:19:44 0|BlueMatt|the code diff is ~trivial
149 2018-05-10 19:19:56 0|morcos|i'm against the backport, but i'm often against backports. i think they don't get enough review, so they need to be really justified to do them
150 2018-05-10 19:20:07 0|wumpus|but normally I'd prefer not to backport things that are not bugfixes, or required to support bugfixes
151 2018-05-10 19:20:30 0|MarcoFalke|So let's leave it for now only in master, seems to be the conclusion
152 2018-05-10 19:20:31 0|BlueMatt|I do like having more reason for miners to upgrade to 0.16.1
153 2018-05-10 19:20:52 0|wumpus|MarcoFalke: yes
154 2018-05-10 19:21:31 0|MarcoFalke|Alright, other topics?
155 2018-05-10 19:21:51 0|BlueMatt|cfields: had one
156 2018-05-10 19:22:22 0|wumpus|#topic one big malloc (cfields)
157 2018-05-10 19:22:43 0|cfields|sipa and I have discussed this briefly, I thought it might be helpful to have a quick discussion here
158 2018-05-10 19:22:43 0|sipa|i like working on that, after #13062
159 2018-05-10 19:22:45 0|gribble|https://github.com/bitcoin/bitcoin/issues/13062 | Make script interpreter independent from storage type CScript by sipa ÷ Pull Request #13062 ÷ bitcoin/bitcoin ÷ GitHub
160 2018-05-10 19:22:50 0|cfields|because there are a few different approaches
161 2018-05-10 19:22:58 0|BlueMatt|yea, I think thats the One Big motivation for 13062
162 2018-05-10 19:23:20 0|wumpus|one big alloc for the entire process?
163 2018-05-10 19:23:21 0|cfields|sipa: is your plan to move to Spans inside of CTransaction?
164 2018-05-10 19:23:36 0|cfields|with a pool for the script data tacked onto the block somewhere?
165 2018-05-10 19:23:57 0|sipa|maybe, there are a lot of possibilities
166 2018-05-10 19:24:05 0|BlueMatt|without it 13062 makes less sense than just haveing a ScriptSig and ScriptPubKey types
167 2018-05-10 19:24:06 0|cfields|wumpus: one malloc for script data per-block
168 2018-05-10 19:24:07 0|cfields|or so
169 2018-05-10 19:24:08 0|sipa|but 13062 is a prerequisite
170 2018-05-10 19:24:15 0|wumpus|2cf
171 2018-05-10 19:24:22 0|wumpus|cfields: ok!
172 2018-05-10 19:24:25 0|sipa|(and more; there are a lot.of things that at CScript specific that shouldn't)
173 2018-05-10 19:24:41 0|cfields|well another option is std::allocator magic, without having to switch to Span
174 2018-05-10 19:24:48 0|wumpus|yes happy to see "span" hapening
175 2018-05-10 19:24:53 0|wumpus|noooo
176 2018-05-10 19:25:02 0|BlueMatt|heh, that would be a lot less code for similar results........
177 2018-05-10 19:25:06 0|wumpus|no magic
178 2018-05-10 19:25:30 0|BlueMatt|I mean you're gonna have similar levels of magic (with more layer violations) to deserialize a whole block into one pool
179 2018-05-10 19:25:33 0|wumpus|yes, but much less easy to understand code
180 2018-05-10 19:26:05 0|cfields|wumpus: i can't argue with that, it looks like voodoo
181 2018-05-10 19:26:21 0|sipa|damn cool voodoo.
182 2018-05-10 19:26:23 0|sipa|but voodoo.
183 2018-05-10 19:26:24 0|wumpus|c++ is already too much voodoo
184 2018-05-10 19:26:31 0|BlueMatt|true, fuck voodoo
185 2018-05-10 19:26:39 0|sipa|let's switch to BASIC
186 2018-05-10 19:26:59 0|BlueMatt|though absolutely NACK 13062 unless there is demonstrated branch that is realisticly-mergeable that uses it
187 2018-05-10 19:27:00 0|cfields|anyway, it got me wondering if it'd be worthwhile to change the p2p format to be more agreeable with allocations
188 2018-05-10 19:27:09 0|sipa|BlueMatt: :(
189 2018-05-10 19:27:11 0|cfields|(next time we're changing something that is, not for this alone)
190 2018-05-10 19:27:18 0|sipa|BlueMatt: i completely disagree :)
191 2018-05-10 19:27:44 0|BlueMatt|and with C++ very easy to fuck up references and have them break
192 2018-05-10 19:27:44 0|jonasschnelli|cfields: can you make an example for the p2p allocation issue?
193 2018-05-10 19:27:46 0|sipa|not everything will become a span?
194 2018-05-10 19:27:56 0|sipa|spans are only used temporarily
195 2018-05-10 19:28:05 0|cfields|BlueMatt: how so? Seems absolutely necessary to me if we're ever going to untangle our subsystems
196 2018-05-10 19:28:13 0|jonasschnelli|cfields: you mean things like inv size comes before the acctual inv data?
197 2018-05-10 19:28:15 0|wumpus|I am a fan of making everything a span, at least for the inner processing, obviously not to keep hold of the data
198 2018-05-10 19:28:20 0|BlueMatt|sure, of course, but I see no reason to be moving the script interpreter to a generic Span thing vs using a CScript type unless there is demonstrated use
199 2018-05-10 19:28:21 0|sipa|exactly
200 2018-05-10 19:28:24 0|cfields|jonasschnelli: right
201 2018-05-10 19:28:30 0|sipa|it's abstracting representation from processing
202 2018-05-10 19:28:39 0|BlueMatt|why? if its just operating on a CScript, it should just operate on a CScript
203 2018-05-10 19:28:45 0|wumpus|I've always found that wedding CScript to a specific backing storage type was a bad idea
204 2018-05-10 19:28:48 0|jonasschnelli|Stumbled over this today.
205 2018-05-10 19:29:05 0|BlueMatt|I mean you can template it if you want :p
206 2018-05-10 19:29:19 0|wumpus|it implies lots of extra allocations and copying
207 2018-05-10 19:29:36 0|BlueMatt|sipa: that was obviously a joke
208 2018-05-10 19:29:36 0|cfields|BlueMatt: if something is just dumb bytes, why not represent it that way?
209 2018-05-10 19:29:43 0|sipa|BlueMatt: so was my jumping :)
210 2018-05-10 19:29:44 0|BlueMatt|its not just dumb bytes
211 2018-05-10 19:29:45 0|luke-jr|wumpus: enough to actually be an issue?
212 2018-05-10 19:29:55 0|sipa|a script is just dumb bytes
213 2018-05-10 19:30:09 0|sipa|any sequence of bytes is a valid scriptPubKey at least
214 2018-05-10 19:30:11 0|wumpus|right - it's simply a span of bytes
215 2018-05-10 19:30:28 0|BlueMatt|yea, I mean ok, sure, dont disagree in principal, but I /really/ hate taking references to things in C++
216 2018-05-10 19:30:39 0|sipa|ok
217 2018-05-10 19:30:43 0|cfields|also, I love the idea of a MultiSpan for batched operations
218 2018-05-10 19:30:44 0|BlueMatt|oops, something realloc'd and now we're executing uninitialized memory
219 2018-05-10 19:31:04 0|sipa|BlueMatt: c++ already uses references for ~everything
220 2018-05-10 19:31:27 0|BlueMatt|kinda, at least you're taking an iterator and its clear whats going on, introduce a new type and we start passing them around in the script executor and.....
221 2018-05-10 19:31:28 0|wumpus|cfields: scatter gather lists?
222 2018-05-10 19:31:42 0|BlueMatt|when its Execute(script, script) you know the thing is fine cause you're giving it a reference to the byte storage
223 2018-05-10 19:32:01 0|cfields|wumpus: right.
224 2018-05-10 19:32:02 0|BlueMatt|not an object that holds a refernece to it that was created sometime in the past
225 2018-05-10 19:32:08 0|sipa|and now it will be Execute(Span(script), Span(script))
226 2018-05-10 19:32:09 0|BlueMatt|cfields: can we not?
227 2018-05-10 19:32:13 0|sipa|same thing.
228 2018-05-10 19:32:18 0|cfields|BlueMatt: hmm?
229 2018-05-10 19:32:39 0|BlueMatt|no, it'll be push_to_background_thread(Span(script), Span(script))
230 2018-05-10 19:32:49 0|sipa|maybe
231 2018-05-10 19:32:59 0|BlueMatt|and more layers makes it less easy to see exactly whats going on
232 2018-05-10 19:33:08 0|wumpus|you can still copy of spans are the API
233 2018-05-10 19:33:11 0|wumpus|it doesn't restrict you in that
234 2018-05-10 19:33:22 0|BlueMatt|hmm?
235 2018-05-10 19:33:22 0|wumpus|it just gives the *option* to use a direct memory buffer
236 2018-05-10 19:33:25 0|sipa|you'd pass script down everything
237 2018-05-10 19:33:33 0|cfields|fwiw, it'd be possible to add an aliasing shared_ptr into Span (or a different SafeSpan maybe) which would keep its owner from deallocating
238 2018-05-10 19:33:33 0|wumpus|just copy a vector and make a span to it
239 2018-05-10 19:33:35 0|sipa|and only when invoking the execution logic you create a span
240 2018-05-10 19:33:36 0|BlueMatt|yes, I get it, and I like the option...when we have a user
241 2018-05-10 19:33:40 0|cfields|that seems wrong though.
242 2018-05-10 19:34:15 0|sipa|anyway, i'd like to start by making scriotSig a different type than scriptPubKey
243 2018-05-10 19:34:18 0|BlueMatt|I mean I'm fine with a refactor that doesnt change the exposed interface as step 1
244 2018-05-10 19:34:19 0|wumpus|I really don't see why not to do it, but anyhow
245 2018-05-10 19:34:28 0|sipa|there are a few more changes to do before that is possible
246 2018-05-10 19:34:35 0|BlueMatt|and have a wrapper in executor.cpp that just calls EvalScript(Span(arg1), Span(arg2))
247 2018-05-10 19:34:43 0|sipa|as the scriot executiin is not the only thing we do with scripts
248 2018-05-10 19:34:48 0|Odin|anybody has a cfw for antminer s5 or s7?
249 2018-05-10 19:34:58 0|BlueMatt|also, it'd be nice to have a type check on ScriptSig/ScriptPUbKey types in the public interface :p
250 2018-05-10 19:35:01 0|sipa|Odin: not here, go away
251 2018-05-10 19:35:08 0|wumpus|Odin: I've replied to this before, go away
252 2018-05-10 19:35:27 0|cfields|BlueMatt: type check?
253 2018-05-10 19:35:38 0|Odin|where I can find that, what room you suggest?
254 2018-05-10 19:35:45 0|BlueMatt|cfields: well if we make scriptpubkey + scriptsig different types, you cant pass them in the wrong order to EvalScript :p
255 2018-05-10 19:35:45 0|jonasschnelli|Odin: #bitcoin
256 2018-05-10 19:36:17 0|cfields|...I'm pretty sure you'd know quickly enough
257 2018-05-10 19:36:47 0|BlueMatt|the libbitcoinconsensus wrapper in rust was the wrong way around for like 3 months, and had users :p
258 2018-05-10 19:36:54 0|BlueMatt|(turns out that mostly just results in it always returning true)
259 2018-05-10 19:36:59 0|sipa|"users"
260 2018-05-10 19:37:43 0|sipa|anyway, no reason to discuss this further i think
261 2018-05-10 19:37:46 0|cfields|completely thread/memory safe though :p
262 2018-05-10 19:38:03 0|sipa|there will be reasons that type/execution separation will be useful for
263 2018-05-10 19:38:12 0|cfields|agreed, thanks. Just wanted a bit of general discussion around it.
264 2018-05-10 19:38:20 0|sipa|(prevector for scriotSig is reaaally bad)
265 2018-05-10 19:39:19 0|wumpus|#topic review coordination (jimpo)
266 2018-05-10 19:40:02 0|jimpo|so this might just be clarifying what hi pri is
267 2018-05-10 19:40:10 0|jimpo|my understanding is it's blockers on longer term things
268 2018-05-10 19:40:17 0|jimpo|and mostly more established contributors
269 2018-05-10 19:40:17 0|wumpus|that's the intent, yes
270 2018-05-10 19:40:35 0|jimpo|I think there's space for another list of things that have been concept ack'ed for people to review
271 2018-05-10 19:40:54 0|jimpo|so that no everyone is reviewing different stuff and there's some actual coordination
272 2018-05-10 19:40:54 0|sipa|bitcoinacks.com ? :)
273 2018-05-10 19:41:06 0|jimpo|Yes, bitcoinacks.com is great
274 2018-05-10 19:41:22 0|jimpo|I'm curious if people think there should be more of a process around this
275 2018-05-10 19:41:24 0|wumpus|the current list is already not working well, I don't really want to add another project
276 2018-05-10 19:41:30 0|wumpus|yes bitcoinacks is great
277 2018-05-10 19:41:31 0|sipa|but yes, i agree - having a better overview on what is concept acks (anf similarly, encouraging people to concept ack/nack quickly) would be good
278 2018-05-10 19:41:36 0|BlueMatt|apparently it doesnt distinguish between nacks and acks
279 2018-05-10 19:41:40 0|sipa|ha.
280 2018-05-10 19:41:45 0|jonasschnelli|pf.
281 2018-05-10 19:41:58 0|wumpus|lol that's an interesting bug
282 2018-05-10 19:42:12 0|sipa|"nack" "nack" "nack" "merged!"
283 2018-05-10 19:42:15 0|jonasschnelli|who maintains bitcoinacks.com?
284 2018-05-10 19:42:21 0|BlueMatt|pierre, apparently
285 2018-05-10 19:42:28 0|jonasschnelli|pierre r.?
286 2018-05-10 19:42:31 0|BlueMatt|yes
287 2018-05-10 19:42:31 0|jimpo|yeah
288 2018-05-10 19:42:54 0|jimpo|wumpus: I have thoughts, but why do you say the current list isn't working well?
289 2018-05-10 19:43:01 0|jonasschnelli|Lets try to get some feature in. I think an additional layer over github would really help
290 2018-05-10 19:43:18 0|BlueMatt|jimpo: it generates very little actual review
291 2018-05-10 19:43:28 0|jonasschnelli|jimpo: people do not review it "with high priority"
292 2018-05-10 19:43:43 0|jonasschnelli|I think this is also because of the nature of OSS
293 2018-05-10 19:43:44 0|wumpus|jimpo: this comes up every meeting, so I don't particularly like to discuss it, but it doesn't attract very much review. Also because the things that end up there tend to be large, complex, hard to review changes.
294 2018-05-10 19:44:01 0|jimpo|so does that mean 1) people don't want to review 2) people don't want to review large/hard changes 3) the list isn't actually hi pri?
295 2018-05-10 19:44:10 0|wumpus|yes, it's the nature of OSS, people can review or not review whatever they want
296 2018-05-10 19:44:30 0|BlueMatt|also reviewing small things is easier, and we've had an influx of small prs over the past 6+ months
297 2018-05-10 19:44:35 0|jonasschnelli|People want to review it,.. but maybe have other priorities?
298 2018-05-10 19:44:58 0|jimpo|I realize this comes up a lot, but that's because it's a big problem IMO
299 2018-05-10 19:45:21 0|wumpus|jonasschnelli: they might just not understand the change well enough
300 2018-05-10 19:45:30 0|jimpo|the solution might be just reducing the number of large changes in the hi pri list and prioritizing smaller ones that are more likely to get merged
301 2018-05-10 19:45:32 0|promag|jimpo: 4) some require deep understanding 5) usually UI stuff has fewer reviews
302 2018-05-10 19:45:35 0|BlueMatt|its not clear there's a solution, the problem is motivation and time to review big changes, and more lists doesnt help with that
303 2018-05-10 19:45:35 0|jonasschnelli|wumpus: and that...
304 2018-05-10 19:45:51 0|BlueMatt|jimpo: even weeks with fewer entries dont get more review
305 2018-05-10 19:46:30 0|jimpo|What if it was a list of 1?
306 2018-05-10 19:46:38 0|wumpus|maybe more entries is better -> not everyone is able to review the same things!
307 2018-05-10 19:47:02 0|jimpo|wumpus: yeah, true...
308 2018-05-10 19:47:38 0|jimpo|My main point is that if everyone is reviewing different stuff because there's no coordination, then reviews go to waste
309 2018-05-10 19:47:39 0|wumpus|jimpo: then what? and who decides anyway? the point is now that everyone can have a PR on there that's blocking them, that should foster cooporation
310 2018-05-10 19:47:47 0|MarcoFalke|Just as a side note: I try to respect the high priority pull requests when I merge things and avoid merging if they create a merge conflict with one of them
311 2018-05-10 19:49:29 0|jimpo|How do other people decide what to review if it's not on the hi pri list?
312 2018-05-10 19:49:33 0|wumpus|jimpo: you could ask someone to review your PR and you'd review theirs
313 2018-05-10 19:49:51 0|wumpus|depending on what they find interesting or they see as their part of the code
314 2018-05-10 19:50:10 0|sipa|jimpo: i generally sort by recently modified
315 2018-05-10 19:51:04 0|MarcoFalke|Agree with wumpus, currently the best way to get review is to find contributors in your "area" and trade review with them
316 2018-05-10 19:51:20 0|MarcoFalke|And coordinate with them what should go in in what order
317 2018-05-10 19:51:25 0|jonasschnelli|jimpo: also, every contributor has its own "roadmap". The high-prio list was usually "one PR per contributor" to "share the roadmap"
318 2018-05-10 19:51:25 0|promag|what MarcoFalke said, git blame and ping the guys here
319 2018-05-10 19:51:39 0|luke-jr|jimpo: personally, I just browse the PR list
320 2018-05-10 19:52:10 0|achow101|I don't do review :/
321 2018-05-10 19:52:25 0|wumpus|I tend to pay most attention to bugfixes, RPC changes, GUI and networking things
322 2018-05-10 19:52:30 0|promag|achow101: its' great
323 2018-05-10 19:52:36 0|jimpo|OK, I don't have much more to say on the topic other than that I think it would be more helpful if there were more clarity around who needs to review what and what should get review.
324 2018-05-10 19:53:01 0|wumpus|everything on the PR list needs review
325 2018-05-10 19:53:05 0|wumpus|otherwise it should be closed
326 2018-05-10 19:53:09 0|wumpus|or merged
327 2018-05-10 19:53:14 0|jonasschnelli|jimpo: what do you mean with *needs to review" ?
328 2018-05-10 19:53:19 0|jimpo|Personally, I'd just be interested in reviewing whatever needs review, so I try to go through the hi pri list.
329 2018-05-10 19:53:49 0|jimpo|Like, if there's a PR and it won't be merged until at least n of this set of loose owners reviews it.
330 2018-05-10 19:53:58 0|jimpo|then, eventually they need to weigh in
331 2018-05-10 19:54:01 0|wumpus|the point of the PR list is to keep track of what needs review, it's crazy that there's so much on there now, but we can't really help it
332 2018-05-10 19:54:34 0|promag|jimpo: IMO you should try to see all PR's
333 2018-05-10 19:54:35 0|luke-jr|right, all open PRs need review
334 2018-05-10 19:54:40 0|jonasschnelli|Yes. Even a full time reviewer like promag could not bring down the open PR #
335 2018-05-10 19:54:43 0|luke-jr|otherwise they wouldn't be open
336 2018-05-10 19:54:51 0|wumpus|and as said, it helps to ping people that edited the code before you, they probaly know the code
337 2018-05-10 19:55:33 0|BlueMatt|jimpo: I agree, the million-papercut-pr review approach doesnt get us anywhere as a project
338 2018-05-10 19:55:41 0|BlueMatt|reviewing things on the highprio list does, at least to me
339 2018-05-10 19:55:46 0|jimpo|if the entire PR list is valid and there's limited time for reviews and we are not willing to close stuff quickly, then there's just a fundamental problem
340 2018-05-10 19:55:56 0|BlueMatt|I generally try to get through the high prio list once a week, though often fail
341 2018-05-10 19:56:05 0|wumpus|a lot of open source projects don't have PR lists at all, for example the freedesktop projects have a mailing list where patches are posted, and the poster of the patch has the responsibility to cc: people that are relevant for review
342 2018-05-10 19:56:24 0|wumpus|the 'dump everyone on a list' approach of github doesn't scale that well
343 2018-05-10 19:56:33 0|promag|the PR's just sit there waiting for feedback.. sometimes a concept ACK can help push it forward
344 2018-05-10 19:56:35 0|jonasschnelli|at least the have no unicorns. :/
345 2018-05-10 19:56:41 0|jimpo|well, I'm basically proposing a ranking
346 2018-05-10 19:56:50 0|promag|otherwise people keep submitting PR's
347 2018-05-10 19:57:04 0|jonasschnelli|Not sure if a "global" ranking is possible.
348 2018-05-10 19:57:05 0|promag|who ranks?
349 2018-05-10 19:57:07 0|wumpus|jimpo: if you have specific proposals on what to close, feel free to let me know
350 2018-05-10 19:57:10 0|jonasschnelli|Because that would assume we have central planing
351 2018-05-10 19:57:17 0|promag|^
352 2018-05-10 19:57:21 0|jimpo|# of concept acks is a ranking
353 2018-05-10 19:57:28 0|jimpo|which bitcoinacks.com is helpful for
354 2018-05-10 19:57:50 0|promag|but if you only see hp then thats biased..
355 2018-05-10 19:58:33 0|jonasschnelli|Could one say a concept ACK of sipa has the same "value" as one from John Doe?
356 2018-05-10 19:58:58 0|MarcoFalke|That is up to you to decide.
357 2018-05-10 19:58:59 0|wumpus|jonasschnelli: people longer in the projects are seen to hae more expertise
358 2018-05-10 19:59:16 0|MarcoFalke|Assign a ranking to each Concept ACK :p
359 2018-05-10 19:59:20 0|wumpus|if 1000 sybil accounts show up and ack something, we'd obviously not fall for that
360 2018-05-10 19:59:24 0|BlueMatt|whatever, I dont think we're making any more progress this week than we have the last 10 times we've discussed this
361 2018-05-10 19:59:31 0|jonasschnelli|But I kinda understand jimpo. We should extend bitcoinacks and make it flexible in terms of "getting the ranked list".
362 2018-05-10 19:59:39 0|wumpus|I agree, this is too meta of a topic, it's not constructive
363 2018-05-10 19:59:43 0|lightningbot|Log: http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-05-10-19.00.log.html
364 2018-05-10 19:59:43 0|lightningbot|Meeting ended Thu May 10 20:00:24 2018 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
365 2018-05-10 19:59:43 0|lightningbot|Minutes: http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-05-10-19.00.html
366 2018-05-10 19:59:43 0|lightningbot|Minutes (text): http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-05-10-19.00.txt
367 2018-05-10 19:59:43 0|wumpus|#endmeeting
368 2018-05-10 19:59:46 0|promag|I've seen some projects where PR's just automatically close after some inactivity.. then the author can reopen
369 2018-05-10 20:00:13 0|jonasschnelli|Yes. We do that manually...
370 2018-05-10 20:00:19 0|jonasschnelli|But should probably do that more often
371 2018-05-10 20:00:35 0|wumpus|again, if there's anything you think that should be closed, let me know and I'll look at it
372 2018-05-10 20:00:41 0|promag|we have 267 open PR's..
373 2018-05-10 20:00:46 0|wumpus|I don't have the time nor energy to keep on top of the PR list, if that was even humanly possible
374 2018-05-10 20:00:52 0|promag|and counting
375 2018-05-10 20:01:04 0|BlueMatt|wumpus: I dont think it has been for at least 6 months :/
376 2018-05-10 20:01:18 0|luke-jr|maybe auto-close after a week of needing rebase?
377 2018-05-10 20:01:21 0|wumpus|BlueMatt: right
378 2018-05-10 20:01:23 0|MarcoFalke|Is there a way to block people from creating pull requests if they have too many outstanding ones?
379 2018-05-10 20:01:29 0|luke-jr|problem is GitHub doesn't let authors reopen..
380 2018-05-10 20:01:32 0|MarcoFalke|I guess that'd be helpful
381 2018-05-10 20:01:53 0|promag|luke-jr: oh
382 2018-05-10 20:01:57 0|luke-jr|MarcoFalke: "you can't work because others aren't"? :P
383 2018-05-10 20:02:04 0|MarcoFalke|And then a note saying "Please trade review"
384 2018-05-10 20:02:36 0|promag|but that's also unfair, I think that closing should happen because of nacks
385 2018-05-10 20:02:45 0|skeees|RCO - review coin?
386 2018-05-10 20:02:56 0|luke-jr|skeees: I was thinking about joking that..
387 2018-05-10 20:02:58 0|jonasschnelli|heh... do an ICO, quick!
388 2018-05-10 20:03:14 0|luke-jr|it might be helpful if users had an easy way to put bounties on review of PRs they like
389 2018-05-10 20:03:14 0|wumpus|promag: ideally, yes, though if no one is interested in a change, it'd make sense to close it too
390 2018-05-10 20:03:37 0|wumpus|promag: I usually close my own PRs after a while if they don't get review or discussion, but many other people are not doing that
391 2018-05-10 20:03:55 0|luke-jr|of course, then we might get the problem of false reviews to collect it :/
392 2018-05-10 20:03:58 0|promag|wumpus: yeah I also want to close some of mines
393 2018-05-10 20:04:43 0|bitcoin-git|[13bitcoin] 15promag closed pull request #11515: Assert cs_main is held when retrieving node state (06master...06201710-node-state-guard) 02https://github.com/bitcoin/bitcoin/pull/11515
394 2018-05-10 20:05:06 0|MarcoFalke|luke-jr: Then you stop trading reviews with that person ;)
395 2018-05-10 20:05:29 0|luke-jr|as soon as I get a chance, I'm hoping to go through mine and rebase/close as appropriate
396 2018-05-10 20:05:34 0|luke-jr|MarcoFalke: how do you even detect it?
397 2018-05-10 20:05:53 0|promag|wumpus: I still think an explicit ack/nack is better than "timeout"
398 2018-05-10 20:05:53 0|wumpus|I don't think we have to be so afraid of fake reviews by serious devs
399 2018-05-10 20:06:07 0|wumpus|promag: I also think so, but it's not always what you can hope for
400 2018-05-10 20:06:27 0|luke-jr|wumpus: no, just fake reviews from newbie devs after users put a bounty on it
401 2018-05-10 20:07:06 0|wumpus|promag: in open source it's sometimes necessary to be somewhat pushy with changes, promote them, and if still no one is interested well that's it, at some point it makes no sense to keep working on it
402 2018-05-10 20:07:13 0|bitcoin-git|[13bitcoin] 15MarcoFalke closed pull request #12935: Add ProcessOrphans (move-only) (06master...06Mf1804-ProcessOrphans) 02https://github.com/bitcoin/bitcoin/pull/12935
403 2018-05-10 20:07:32 0|promag|wumpus: yeah I usually beg for merge right? :P
404 2018-05-10 20:07:51 0|wumpus|promag: it's easier to get attention for changes that are end-user visible
405 2018-05-10 20:08:08 0|wumpus|e.g. someone really wants a certain RPC call and is willing to test it
406 2018-05-10 20:08:38 0|wumpus|optimizations also tend to attract review quickly
407 2018-05-10 20:08:55 0|luke-jr|I wonder if it would be helpful or harmful to encourage non-devs to test stuff more
408 2018-05-10 20:08:56 0|wumpus|unless the code changes are very complex and big
409 2018-05-10 20:09:46 0|wumpus|luke-jr: we usually do that, for GUI changes
410 2018-05-10 20:09:57 0|BlueMatt|I mean part of my goal with high-prio was "everyone gets to make some progress on one of their pr's once a week", but it seems no one else wanted to use it that way, and that's fine, but unless people have new suggestiotns, its not worth discussing these things anymore, I think
411 2018-05-10 20:10:02 0|BlueMatt|maybe we should have a rule against them at meeting
412 2018-05-10 20:10:04 0|wumpus|but GUI changes are, everything considered, already easy to get merged
413 2018-05-10 20:10:40 0|promag|wumpus: for instance #12578
414 2018-05-10 20:10:42 0|gribble|https://github.com/bitcoin/bitcoin/issues/12578 | Add transaction record type Fee by promag ÷ Pull Request #12578 ÷ bitcoin/bitcoin ÷ GitHub
415 2018-05-10 20:11:11 0|promag|should be merged or closed?
416 2018-05-10 20:11:25 0|wumpus|(test changes are also, usually, either quickly merged or quickly rejected)
417 2018-05-10 20:11:53 0|wumpus|promag: for that it makes a lot of sense to let some non-dev users test
418 2018-05-10 20:12:03 0|wumpus|promag: whether the new transaction list format is desirable/useful
419 2018-05-10 20:12:34 0|wumpus|promag: maybe jonasschnelli can do a build there
420 2018-05-10 20:12:59 0|promag|some non-dev users test <- rarely seen right?
421 2018-05-10 20:13:06 0|wumpus|BlueMatt: I agree
422 2018-05-10 20:13:15 0|wumpus|promag: well try to find someone that is interested in your change!
423 2018-05-10 20:13:50 0|MarcoFalke|Right, GUI changes also need testing from GUI users.
424 2018-05-10 20:13:59 0|luke-jr|promag: well, lots of PRs get into Knots, so it could be a matter of getting those users to report on their issues or lack thereof
425 2018-05-10 20:14:12 0|wumpus|promag: I"m sure you can find people to at least discuss about whether this change is desirable or not?
426 2018-05-10 20:14:38 0|wumpus|promag: it's not some obscure checkbox that moves somewhere else, but the layout of the transaction list, people will care about this
427 2018-05-10 20:17:39 0|promag|wumpus: well I don't really care about that, I just think it makes sense but I also can close it..
428 2018-05-10 20:18:28 0|wumpus|I mean you could ask on reddit, or twitter, or whereever GUI users are, I think the problem on github is that no one really has much opinion on the GUI as long as it keeps working :-)
429 2018-05-10 20:19:47 0|MarcoFalke|On firefox the mobile mode is on CTRL+SHIFT+M
430 2018-05-10 20:19:57 0|MarcoFalke|You can set a custom device
431 2018-05-10 20:20:03 0|MarcoFalke|Or just modify the UA
432 2018-05-10 20:20:08 0|wumpus|promag: I've concept ACK'ed it FWIW
433 2018-05-10 20:20:27 0|luke-jr|wumpus: I use and have an opinion that it's dumb to have 2 lines for every tx :P
434 2018-05-10 20:20:59 0|wumpus|luke-jr: you did concept ACK it, maybe retract that then?
435 2018-05-10 20:21:57 0|luke-jr|Well, it makes sense for sendmanys
436 2018-05-10 20:23:24 0|wumpus|wouldn't expanding the fee only for sendmanys be inconsistent?
437 2018-05-10 20:23:40 0|promag|luke-jr: you think the fee should be "associated" to the destinatino?
438 2018-05-10 20:23:57 0|luke-jr|promag: not really
439 2018-05-10 20:24:04 0|wumpus|I mean it might seem silly but there's certainly some precedent in billing to have separate rows for fees
440 2018-05-10 20:24:39 0|luke-jr|Maybe a checkbox to show/hide fees or something would make sense
441 2018-05-10 20:24:56 0|promag|right, what I think could work well is to aggregate the fee outside the table
442 2018-05-10 20:25:03 0|luke-jr|or even just an extra column
443 2018-05-10 20:25:08 0|wumpus|then again I don't care so deeply I really want to argue either for or against this
444 2018-05-10 20:25:27 0|promag|luke-jr: with extra column and multisend, which row you put the fee?
445 2018-05-10 20:25:34 0|luke-jr|(for tax purposes, I found I have to use JSON-RPC to get useful data)
446 2018-05-10 20:25:41 0|wumpus|would vote against adding extra columns
447 2018-05-10 20:27:47 0|promag|anyway, a clear case of lack of feedback
448 2018-05-10 20:28:47 0|luke-jr|I suppose bank statements do show fees separately - they're just uncommon in general
449 2018-05-10 20:29:03 0|promag|luke-jr: right
450 2018-05-10 20:29:16 0|promag|but my point is consistency with listransactions
451 2018-05-10 20:29:51 0|promag|amounts are different
452 2018-05-10 20:30:03 0|luke-jr|listtransactions doesn't have separate rows for fees
453 2018-05-10 20:31:22 0|promag|right, but it doesn't sum amount and fee
454 2018-05-10 20:31:24 0|Odin|what it's the best pool for btc, not slush, prohashing or antpool
455 2018-05-10 20:31:32 0|promag|bye Odin
456 2018-05-10 20:32:38 0|luke-jr|if we want to imitate listtransactions, that would suggest an extra column (which wumpus doesn't like)
457 2018-05-10 20:32:45 0|luke-jr|wumpus: what if it was hidden by default?
458 2018-05-10 20:33:07 0|wumpus|there are already so many columns, I think adding more would be against user friendly UI design, then again I don't have a dog in that fight
459 2018-05-10 20:33:25 0|wumpus|sure, you could do optional columns
460 2018-05-10 20:40:24 0|luke-jr|how about an option? "Show fees: Not at all; As an extra column; Included in amount"
461 2018-05-10 20:40:41 0|luke-jr|hopefully it will become obvious which of the latter two is preferred with time
462 2018-05-10 20:48:06 0|wumpus|making everything an option because we just can't decide :)
463 2018-05-10 22:12:20 0|bitcoin-git|13bitcoin/06master 145d53661 15John Newbery: [tests] Remove 'account' API from wallet functional tests...
464 2018-05-10 22:12:20 0|bitcoin-git|[13bitcoin] 15MarcoFalke pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/1c582503507b...cb9bbf77726a
465 2018-05-10 22:12:21 0|bitcoin-git|13bitcoin/06master 14cb9bbf7 15MarcoFalke: Merge #13075: [tests] Remove 'account' API from wallet functional tests...
466 2018-05-10 22:13:00 0|bitcoin-git|[13bitcoin] 15MarcoFalke closed pull request #13075: [tests] Remove 'account' API from wallet functional tests (06master...06remove_functional_tests_accounts) 02https://github.com/bitcoin/bitcoin/pull/13075