--- Log opened Thu Jun 27 00:00:54 2019
01:34 -!- belcher [~belcher@unaffiliated/belcher] has joined #rust-bitcoin
02:00 -!- neonknight642 [~neonknigh@195.159.29.126] has joined #rust-bitcoin
02:00 -!- neonknight642 is now known as neonknight64
02:18 -!- jtimon [~quassel@205.201.35.37.dynamic.jazztel.es] has joined #rust-bitcoin
02:19 -!- CjS77 [~caylemeis@195.159.29.126] has joined #rust-bitcoin
02:32 -!- ccdle12 [~ccdle12@cpc139350-aztw33-2-0-cust310.18-1.cable.virginm.net] has joined #rust-bitcoin
02:51 -!- ccdle12 [~ccdle12@cpc139350-aztw33-2-0-cust310.18-1.cable.virginm.net] has quit [Remote host closed the connection]
02:57 -!- ccdle12 [~ccdle12@cpc139350-aztw33-2-0-cust310.18-1.cable.virginm.net] has joined #rust-bitcoin
03:07 -!- ccdle12 [~ccdle12@cpc139350-aztw33-2-0-cust310.18-1.cable.virginm.net] has quit [Remote host closed the connection]
03:14 -!- ccdle12 [~ccdle12@cpc139350-aztw33-2-0-cust310.18-1.cable.virginm.net] has joined #rust-bitcoin
03:22 -!- ccdle12 [~ccdle12@cpc139350-aztw33-2-0-cust310.18-1.cable.virginm.net] has quit [Remote host closed the connection]
03:24 -!- ccdle12 [~ccdle12@cpc139350-aztw33-2-0-cust310.18-1.cable.virginm.net] has joined #rust-bitcoin
07:08 < stevenroose> andytoshi: I replied in the thread, I meant that builder is meant mostly to just convert a struct in "syntax" i.e. opcodes and variable pushes into a `Script`. So all opcodes are known upfront.
07:09 < stevenroose> I thought in Bitcoin use of non-standard scripts was very much discouraged. If you're going to change a variable unknown opcode into another one (if you knew it you could just have used the verify-ified opcode instead) you're most probably not using standard script templates.
07:10 < stevenroose> andytoshi: from_script should be finished except that it doesn't accept non-zero segwit versions, but that's because of bitcoin-bech32, but #255 removes that dependency.
07:11 < stevenroose> sgeisler: is bech32 actually ready for #255? I lost track, sorry :/
07:18 -!- michaelsdunn1 [~michaelsd@unaffiliated/michaelsdunn1] has joined #rust-bitcoin
08:13 < sgeisler> rust-bech32#35 is ready for review, just removed some unused code and waiting for travis. It includes your no-struct refactorings and allocationless encoding
08:16 < sgeisler> stevenroose: would be great if you could review it since it includes subtle bit manipulation (that I described in the comments) and it's really easy to get it wrong imo
08:17 < sgeisler> after that we'd need another PR bumping the version to 0.7.0 and clarkmoody has to release it on crates.io, then #255 should be unblocked imo
08:18 < stevenroose> sgeisler: cool!
08:18 < stevenroose> will look at it in a sec
08:33 -!- DeanGuss [~dean@gateway/tor-sasl/deanguss] has quit [Ping timeout: 260 seconds]
08:58 -!- michaelsdunn1 [~michaelsd@unaffiliated/michaelsdunn1] has quit [Remote host closed the connection]
10:04 -!- stanimal [~stanimal@195.159.29.126] has joined #rust-bitcoin
10:09 -!- michaelsdunn1 [~michaelsd@unaffiliated/michaelsdunn1] has joined #rust-bitcoin
10:45 < ariard> BlueMatt: okay maybe the issue is somewhere when we serialize on disk, when I connect to a c-lightning node without channel on my side, everything is fine, when I relaunch the sample with previous channel data then I get "Malformed channel_announcement"
10:49 < andytoshi> stevenroose: basically all scripts are standard in p2sh
10:50 < andytoshi> there are no "standard script templates"
10:50 < andytoshi> and again, the only way i can know the opcode at the end of a Builder is if i stored it separately when i pushed it
10:50 < andytoshi> and then why would i even be using a builder?
11:09 < stevenroose> andytoshi: I don't understand.. Are you playing devil's advocate?..
11:09 < stevenroose> These are IMO the only legitimate usages of the script builder: https://github.com/rust-bitcoin/rust-bitcoin/blob/master/src/util/address.rs#L187-L192
11:09 < andytoshi> no i'm not. i actually need to use the script builder to add a verify
11:09 < stevenroose> I.e. you know the skeleton and fill in variable pushes.
11:09 < andytoshi> and the last-added opcode was added by some other function, i don't necessarily know which one even
11:09 < andytoshi> because i'm serializing miniscripts
11:10 < stevenroose> If you need the verify-ifyer you're not doing safe scripting, is what I'm saying.
11:10 < andytoshi> you are wrong
11:10 < andytoshi> there is nothing unsafe about compressing a CHECKSIG VERIFY into CHECKSIGVERIFY
11:11 < andytoshi> and there would be nothing safer if my abstract script representations had to carry around extra metadata indicating that they end in some special VERIFY-able opcodes
11:11 < stevenroose> Yeah but if you know the last opcode is a checksig, you can just as well swap it for a checksigverify or just not have pushed checksig but pushed checksigverify..
11:11 < andytoshi> i pushed checksig in the serialization of check(...)
11:11 < stevenroose> Carrying around scripts and modifying them after the creation is IMO also unsafe..
11:11 < andytoshi> and i'm pushing verify in the serialization of verify(...)
11:11 < andytoshi> i'm not modifying them after creation
11:11 < andytoshi> i'm trying to create them
11:11 < stevenroose> Yeah I know it's for miniscript.
11:12 < stevenroose> What I'm saying is that I think I would implement this method in miniscript instead of in rust-bitcoin because it's doing subtle script fiddling normal rust-bitcoin users shouldn't be doing.
11:12 < andytoshi> i can't do it in rust-miniscript
11:12 < andytoshi> because i can't access the inner script
11:12 < andytoshi> are you suggesting i expose the inner Vec of the Builder?
11:13 < stevenroose> Perhaps.. :D
11:13 < andytoshi> rather than adding a safe method that will always have straightforward semantics?
11:13 < BlueMatt> ariard: oh hmmm. do you have a blob or log line?
11:13 < stevenroose> You can't get a mut ref to the inner in some way?
11:13 < andytoshi> i can do that, but it feels much less safe than what i've PR'd
11:13 < BlueMatt> ariard: i wonder if they send something different to their peers
11:13 < BlueMatt> that we get mad about
11:13 < andytoshi> stevenroose: no, because the point is that the Builder will always output a valid script
11:13 < andytoshi> and if i had a mutable accessor that invariant would be lost
11:13 < stevenroose> Yeah true, it's also unsafe. And I agree that it's not sane to require reallocating the byte string for this...
11:14 < stevenroose> Hmm.. pop_opcode?
11:14 < andytoshi> what would that do if the last thing to be pushed was a slice?
11:15 < andytoshi> what if the user did .push_int(1)?
11:15 < stevenroose> None. Is there an Instruction enum with variants opcode and push?
11:15 < andytoshi> no, the Builder has a Vec<u8> inside
11:16 < andytoshi> oh, hmm, i realize that my code is actually incorrect ... you can .push_slice() something that ends in a CHECKSIG byte
11:16 < andytoshi> and it'll modify the slice
11:16 < andytoshi> rather than adding VERIFY
11:16 < stevenroose> Hmm there is Instruction but it doesn't work with pop
11:17 < andytoshi> right. to do this correctly we'd need to instruction-ify the entire script and check the last Instruction
11:17 < andytoshi> or just keep track of the last-pushed opcode
11:18 < andytoshi> either of which can be done inside Builder but would be very inconvenient to do outside of it
11:18 < stevenroose> Well you'd need to do inspection for verify-ify as well as pop_opcode, right?
11:18 < andytoshi> yes
11:18 < stevenroose> or pop_element for that matter
11:18 < stevenroose> I don't think going over a script is that bad though
11:18 < andytoshi> yeah, agreed, it doesn't even allocate
11:18 < stevenroose> I'm not sure what script sizes you're envisioning, though :)
11:19 < stevenroose> 'mini' ones I suppose
11:19 < andytoshi> lol :)
11:19 < andytoshi> i'll probably just maintain a `last_push` cache inside the Builder
11:19 < stevenroose> While pop_opcode also kinda breaks the no unsafe use principle I mentioned, I think it makes a bit more sense, it fits wiht pus hopcode (or pop_element for that matter)
11:20 < andytoshi> pop_element would need to allocate in general
11:20 < andytoshi> to return pushed slices
11:20 < stevenroose> andytoshi: some other script builder impls I've seen just keep a list if Instruction objects and serialize in the end
11:20 < andytoshi> we could also do that
11:20 < andytoshi> except that our Instruction objects don't own their slices
11:20 < andytoshi> and the Builder needs to own them
11:21 < stevenroose> yeah it'd need another one
11:21 < andytoshi> so doing this would involve multiple allocations/deallocations
11:21 < andytoshi> so the user has some data, needs to allocate an Instruction to give that to the Builder, which then needs to allocate just to copy out of the Instruction?
11:21 < stevenroose> well we can always `last_instruction_type` and `pop_opcode() -> Option<opcode>` or something
11:21 < andytoshi> i don't mind adding `.pop_opcode()`
11:22 < andytoshi> but i'm going to do it in an allocation-free way, by just caching the most recent `Instruction` to be pushed
11:22 < stevenroose> yeah keeping the instruction list would mean reallocating on serialization
11:22 < andytoshi> hmm actually i think the borrowck won't let me do that
11:22 < stevenroose> I'd just have quick script inspection
11:22 < stevenroose> and have pop_opcode and pop_push that both return an option in case it couldn't be popped
11:23 < stevenroose> then you don't need an instruction type and op_push cna just be Option<Vec<u8>>
11:25 < andytoshi> yep exactly
11:26 < andytoshi> i have a call nowish, need an hour before i can fix th PR
11:42 < ariard> BlueMatt: hmmm to be more accurate it also happens when we are connected to 2 c-lightning nodes, when we connect to 2nd node, we start to send to it channel_announcement/updates we get from 1st
11:42 < ariard> and then the 2nd one disconnect us
11:44 < BlueMatt> oh wut
11:44 < BlueMatt> so they think we're sending garbage, too
11:45 < ariard> https://gist.github.com/ariard/2ce7472ef1d8c6b4c05cc394c10057f3
11:47 < ariard> yeah the issue should be in our InitialSync sequence
12:37 -!- DeanGuss [~dean@gateway/tor-sasl/deanguss] has joined #rust-bitcoin
12:57 -!- DeanGuss [~dean@gateway/tor-sasl/deanguss] has quit [Ping timeout: 260 seconds]
13:35 -!- DeanGuss [~dean@gateway/tor-sasl/deanguss] has joined #rust-bitcoin
13:37 -!- DeanGuss [~dean@gateway/tor-sasl/deanguss] has quit [Client Quit]
13:37 -!- DeanGuss [~dean@gateway/tor-sasl/deanguss] has joined #rust-bitcoin
14:38 -!- ccdle12 [~ccdle12@cpc139350-aztw33-2-0-cust310.18-1.cable.virginm.net] has quit [Remote host closed the connection]
15:11 < sgeisler> > 8:16 PM <andytoshi> oh, hmm, i realize that my code is actually incorrect ... you can .push_slice() something that ends in a CHECKSIG byte
15:11 < sgeisler> Oh, wow, that's subtle, completely missed it :(
15:22 -!- michaelsdunn1 [~michaelsd@unaffiliated/michaelsdunn1] has quit [Remote host closed the connection]
16:38 -!- DeanGuss [~dean@gateway/tor-sasl/deanguss] has quit [Ping timeout: 260 seconds]
18:10 -!- reallll [~belcher@unaffiliated/belcher] has joined #rust-bitcoin
18:13 -!- belcher [~belcher@unaffiliated/belcher] has quit [Ping timeout: 245 seconds]
19:02 < andytoshi> stevenroose: can you take a look at https://github.com/rust-bitcoin/bitcoin_hashes/pull/51 ?
19:10 -!- TamasBlummer [~Thunderbi@p200300DD67196B4991D1696F4001E110.dip0.t-ipconnect.de] has quit [Ping timeout: 264 seconds]
19:11 -!- TamasBlummer [~Thunderbi@p200300DD6712645891D1696F4001E110.dip0.t-ipconnect.de] has joined #rust-bitcoin
20:31 -!- jtimon [~quassel@205.201.35.37.dynamic.jazztel.es] has quit [Ping timeout: 245 seconds]
23:23 -!- BlueMatt [~BlueMatt@unaffiliated/bluematt] has quit [Ping timeout: 252 seconds]
23:28 -!- BlueMatt [~BlueMatt@unaffiliated/bluematt] has joined #rust-bitcoin
23:52 -!- dpc [dpcmatrixo@gateway/shell/matrix.org/x-nxvbsfmelniygynb] has quit [Remote host closed the connection]
--- Log closed Fri Jun 28 00:00:55 2019