c-lightning blessed branch - #208
Closed
niftynei wants to merge 37 commits into
Closed
Conversation
AFAICT, the below does the same thing, and avoids glslang having to tell me to add my test there. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As revealed by: make && LD_LIBRARY_PATH=`pwd`/src/.libs valgrind --leak-check=full src/.libs/test_psbt There are more, but the next patches rework that code. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Manually checking for end of buffers is error prone: these pull & push variants are similar to those used by c-lightning. Importantly, if they fail you can still call them, meaning you can check whether it overflowed at the end of all the work. The push function also can tell you exactly how many bytes short you were. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This also means we return WALLY_EINVAL instead of corrupting the stack if they provide a witness output > 50 bytes. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As a special bonus, document that it now returns bytes_written as the entire length, if the error was caused by bytes_len being insufficient. This allows the caller to avoid calling bytes_len in the common case, if they want to. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…ytes. This avoid (recently buggy!) duplication. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Now we pull the magic before calling count_psbt_parts(), it no longer needs to skip over it. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This currently fails (badly). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
valgrind is better, but this is simple and finds current leaks. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Byte vectors in a PSBT could have a 0 length and thus result in duplicate keys in a PSBT as length 0 is also assumed that there is no vector for that field. Instead of checking length, check that the pointer is non-null. To ensure the pointers are non-null, malloc 1 byte. The length is still stored as 0.
wally_psbt_elements_init_alloc is added to set an elements magic wally_psbt_init_alloc, wally_psbt_to_bytes, and wally_psbt_from_bytes are updated to deal with this magic instead of the hard coded magic. wally_psbt_is_elements is added.
so it needs the bool header
libwally wants you to flag in what 'type' of transaction it's handling in a few places (in others it seems to be able to figure it out for itself)
elementsd serializes the asset, value, nonce, and scriptpubkey for a 'witness_utxo' on the input; we were just serializing the amount + scriptpubkey, which made libwally psbt's incompatible with elementsd originated psets.
We were pulling out a subset of the key for the subtype, which wasn't correct. What we really want to do is finish using the already pulled 'key' object, which encompasses all of the subtype info (subtypes don't have length prefixes -- this might be a bug in elementsd?)
was breaking travis
We were forgetting to add the subtype for proprietary element's keys
…LUEs Small oversight, omitted the length of the amount value when serializing
An unsigned, parseable PSET that we now pass test against
probably should be swallowed into originating problem commit
Needs the _DEFAULT_SOURCE flag? Recently upgraded to Ubuntu20.04 which can't find MAP_ANON. I traced the problem to __USE_MISC not being flagged on, for who knows why. Some digging led me to this solution. Some further patches from Joseph complete the deprecation of _BSD_SOURCE and _SVID_SOURCE for the upcoming glibc 2.20 release. The first of these patches converts _BSD_SOURCE and _SVID_SOURCE into synonyms for _DEFAULT_SOURCE. The deprecated macros remain effective, but cause compile-time warnings to be generated if they are used (unless _DEFAULT_SOURCE is also defined). The second patch (mechanically) converts all of the uses of the __USE_SVID and __USE_BSD internal macros in glibc header files into __USE_MISC. Source: https://lwn.net/Articles/590381/
During PSBT preparation, it can reasonably contain unfinished transactions. In particular, we didn't allow marshalling of zero-output transactions. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Edited-By: niftynei <niftynei@gmail.com>
@niftynei needs it for the clightning things
niftynei
force-pushed
the
nifty/blessed-branch
branch
from
July 12, 2020 22:33
381f36f to
1f45aef
Compare
Contributor
|
Hi @niftynei, thanks for bringing these together. Could you check out https://github.com/jgriffiths/libwally-core/tree/review_updates please? It should have all the changes you require from here plus a significant number of other fixes. There are some API changes in some cases you will need to account for, listed in CHANGES.md in that branch. Some of the changes from yourself and Rusty in this PR were also modified slightly, if you let me know anything that's missing or still broken for you in the comments here, I'll get them fixed/merged ASAP. I hope to PR this branch later today or tomorrow. And then close most outstanding issues/PRs. Thanks! |
Contributor
|
Actually, having looked at the c-lightning code using this, it might be best if I make the changes to move to this branch post merging. Can you confirm that c-lightnings master branch is the one I should attempt to update? |
This was referenced Jul 13, 2020
Merged
Contributor
|
Closing this in favour of #210 which includes/supersedes it. Please re-open if something you need got missed in the merge. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Set of patches used to get PSETs (and, minorly, partial txs) working in c-lightning. Posted here as reference.
Combines patches from #206, #201, and #191, as well as another set of patches to fix found errors in the outstanding PSET serialization PR.