Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build release docs #13

Closed
wants to merge 565 commits into from
Closed

Build release docs #13

wants to merge 565 commits into from

Conversation

ShahanaFarooqui
Copy link

No description provided.

rustyrussell and others added 30 commits April 7, 2023 11:49
…ons.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…ons.

This breaks tests/test_closing.py::test_onchain_all_dust's accouting
checks.

That test doesn't really test what it claims to test; sure, onchaind
*says* it's going to ignore the output due to high fees, but the tx
still gets mined.

I cannot figure out what the test is supposed to look like, so I
simply disabled the accounting checks :(

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is when they closed the channel, we can simply make our own tx to
expire the HTLC.  (The other case is where we closed the channel, and
we have a special htlc_timeout tx which we have their signature for).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…aind_broadcast

Using single tuples in Python is ugly, so:
1. Rename wait_for_onchaind_tx to wait_for_onchaind_txs.
2. Make it take tuples explicitly.
3. Make wait_for_onchaind_tx a simpler wrapper/unwrapper.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We do this for HTLCs which will timeout to them: we watch them in case we
want to fulfill them as a preimage comes in, but once they reach depth we
can forget about them.

We change the message, which causes some more test churn.

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>
Inside our integration testing we get another timeout,
so this commit adds a timeout to the waitpay command to avoid waiting forever.

Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
It's in the main bolt, so remove qualifies from the bolt quote so they
are checked.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Too much context, it didn't apply if you try to rebuild from source!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
"BOLT 4: Remove legacy format, make var_onion_optin compulsory."

This also renamed the redundant "tlv_payload" to "payload", so we
replace "tlv_tlv_payload" with "tlv_payload" everyhere!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
"Allow nodes to overshoot the MPP `total_msat` when paying (ElementsProject#1031)"

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Protocol: Allow slight overpaying, even with MPP, as spec now recommends.
"Allow nodes to overshoot final htlc amount and expiry (ElementsProject#1032)"

Note that this also renamed `min_final_cltv_expiry` to the more-correct
`min_final_cltv_expiry_delta`.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
"BOLT 04: remove associated data from test vector"

(We actually use merge point).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
"BOLT 03: fix static-remote same amt and pre-image test vector"

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In particular:
	- Bolt 4: add route blinding construction
	- Bolt 4: add blinded payments

And this means it's not experimental, so we can turn it on
by default!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: blinded payments are now supported by default (not just with `--experimental-onion-messages`)
This is for VLS, which doesn't implement signing for the non-standard
recurrence fields.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
They prefer Paths to be explicitly cast as strings
This should have been added earlier as @cdecker suggested, but is needed
to enable CI testing.

Changelog-Changed: Reckless - added support for networks beyond bitocoin and regtest
This will be used during CI testing in the following commit.
A canned lightningd/plugins is used to test against. This allows faster
and more deterministic outcomes.

Changelog-None
These corpora were generated with default libFuzzer flags with 30+ hours
of CPU time, and then minimized with:
  ./fuzz-TARGET -merge=1 -shuffle=0 -prefer_small=1 -use_value_profile=1 corpora/fuzz-TARGET UNMINIMIZED_CORPUS
The following arguments were copied from Bitcoin Core's corpus merging
script https://github.com/bitcoin/bitcoin/blob/master/test/fuzz/test_runner.py:

-shuffle=0
-prefer_small=1
-use_value_profile=1
The script runs each fuzz target on its seed corpus and prints any
failures.
The target builds and runs each fuzz target on its seed corpus.
We want to encourage contributions to the seed corpora that improve
coverage.
Adding a new field with `added` fails:

```
AssertionError: Field Feerates.perkb.estimates[] does not have an `added` annotation
```

Looks like this assertion is wrong: we should get an added from the field itself or
from the .msggen.json file.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell and others added 28 commits June 5, 2023 09:25
We now know the base reference, and we've rebased, so we can do a
simple diff.  Also, this means we can use a magic commit message
`No-schema-diff-check` to suppress false positives.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Detected by UBSan:

$ UBSAN_OPTIONS=print_stacktrace=1 ./wallet/test/run-psbt_fixup

bitcoin/psbt.c:733:2: runtime error: applying zero offset to null pointer
    #0 0x53c829 in psbt_from_bytes lightning/bitcoin/psbt.c:733:2
    #1 0x5adcb0 in main lightning/wallet/test/run-psbt_fixup.c:174:10

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior bitcoin/psbt.c:733:2
The function is tiny and was only used in one location. And that one
location was leaking memory.

Detected by ASan:

==2637667==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 7 byte(s) in 1 object(s) allocated from:
    #0 0x4cd758 in __interceptor_strdup
    #1 0x64c70c in json_stream_log_suppress_for_cmd lightning/lightningd/jsonrpc.c:597:31
    #2 0x68a630 in json_getlog lightning/lightningd/log.c:974:2
    ...

SUMMARY: AddressSanitizer: 7 byte(s) leaked in 1 allocation(s).
It is possible for db_column_bytes() to return 0 and for
db_column_blob() to return NULL even when db_column_is_null() returns
false. We need to short circuit in this case.

Detected by UBSan:

  db/bindings.c:479:12: runtime error: null pointer passed as argument 2, which is declared to never be null
  /usr/include/string.h:44:28: note: nonnull attribute specified here

  #0 0x95f117 in db_col_arr_ db/bindings.c:479:2
  #1 0x95ef85 in db_col_channel_type db/bindings.c:459:32
  #2 0x852c03 in wallet_stmt2channel wallet/wallet.c:1483:9
  #3 0x81f396 in wallet_channels_load_active wallet/wallet.c:1749:23
  #4 0x81f03d in wallet_init_channels wallet/wallet.c:1765:9
  #5 0x72f1f9 in load_channels_from_wallet lightningd/peer_control.c:2257:7
  #6 0x672856 in main lightningd/lightningd.c:1121:25
Otherwise we later copy the uninitialized memory to descendants,
triggering undefined behavior:

plugins/libplugin-pay.c:2882:34: runtime error: load of value 190, which is not a valid value for type 'bool'
Fixes nullability errors detected by UBSan:

wire/fromwire.c:173:46: runtime error: null pointer passed as argument 1, which is declared to never be null
external/libwally-core/src/secp256k1/include/secp256k1.h:432:3: note: nonnull attribute specified here
    #0 0x65214a in fromwire_secp256k1_ecdsa_signature wire/fromwire.c:173:6
    #1 0x659500 in printwire_secp256k1_ecdsa_signature devtools/print_wire.c:331:1
    #2 0x646ba2 in printwire_channel_update wire/peer_printgen.c:1900:7
    #3 0x637182 in printpeer_wire_message wire/peer_printgen.c:128:11
    #4 0x65a097 in main devtools/decodemsg.c:85:10
Memory leak detected by ASan:

==880002==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 32816 byte(s) in 1 object(s) allocated from:
    #0 0x5039e7 in malloc (lightningd/lightningd+0x5039e7)
    #1 0x7f2e8c203884 in __alloc_dir (/lib64/libc.so.6+0xd2884)
This is a simplification for our build system
that allows for managing and cleaning the external
build artefacts in an easy way.

We have a hard time with the suffix `-modded` inside the
version with some architecture when building a tagged version.

Link: https://discord.com/channels/899980449231814676/899989729183940629/1110957992158965770
Reported-by: @ctrlbreak-
Suggested-by: Rusty Russell <rusty@rustcorp.com.au>
Co-developed-by: @jsarenik
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
This PR:
- adds all the guides (in markdown format) that is published at https://docs.corelightning.org/docs
- adds a github workflow to sync any future changes made to files inside the guides folder
- does not include API reference (json-rpc commands). Those will be handled in a separate PR since they're used as manpages and will require a different github workflow

Note that the guides do not exactly map to their related files in doc/, since we reorganized the overall documentation structure on readme for better readability and developer experience. For example, doc/FUZZING.md and doc/HACKING.md#Testing are merged into testing.md in the new docs. As on the creation date of this PR, content from each of the legacy documents has been synced with the new docs. Until this PR gets merged, I will continue to push any updates made to the legacy documents into the new docs.

If this looks reasonable, I will add a separate PR to clean up the legacy documents from doc/ (or mark them deprecated) to avoid redundant upkeep and maintenance.

Changelog-None
This lead to confusion about how to specify `amount_msat`.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Previously any attempt would result in "is not an integer field"; we
now recognize valid JSON integers as integer fields.

Changelog-Fixed: Plugins: `commando` runes can now compare integer parameters using '<' and '>' as expected.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It sometimes takes > 1800 seconds under valgrind.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
defaults were deprecated in 0df9754, but that was a bit
harsh as several plugins do that (summary, for example).  So allow false, but warn
that we ignore anything else.

Reported-by: @microsatoshi on Discord.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: Plugins: ...actually, `default` `false` still accepted on `flag` type parameters.
Detected by ASan in test_hsmtool_generatehsm:

==58698==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 120 byte(s) in 1 object(s) allocated from:
    #0 0x4e6247 in malloc
    #1 0x7f078452d672 in getdelim

SUMMARY: AddressSanitizer: 120 byte(s) leaked in 1 allocation(s).
Running with sanitizers in CI will help prevent us from introducing new
memory safety errors.
Running with sanitizers in CI will help prevent us from introducing new
memory safety errors.
Due to a security report by github, we should increase
our cryptography lib version.

This may impact potential another version that is stuck
with a cryptography version.

Link: ElementsProject#6164
Reported-by: @dni
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
option_scid_alias inside a channel_type allows for more private
channels: in particular, it tells the peer that it MUST NOT allow
routing via the real short channel id, and MUST use the alias.

It only makes sense (and is only permitted!) on unannounced channels.

Unfortunately, we didn't set this bit in the channel_type in v12.0
when it was introduced, instead relying on the presence of the feature
bit with the peer.  This was fixed in 23.05, but:

1. Prior to 23.05 we didn't allow it to be set at all, and
2. LND has a limited set of features they allow, and this isn't allowed without
   option_anchors_zero_fee_htlc_tx.

We could simply drop this channel_type until we merge anchors, *but*
that has nasty privacy implications (you can probe the real channel id).

So, if we don't negotiate anchors (we don't!), we don't set this
channel_type bit even if we want it, and *intuit* it, based on:

1. Is this a non-anchor channel_type?
2. Did we both send channel_type?
3. Is this an unannounced channel?
4. Did both peers announce support for scid aliases?

In addition, while looking at the previous backwards-compat code, I
realized that v23.05 violated the spec and send accept_channel with
OPT_SCID_ALIAS if it intuited it, even if it wasn't offered.  Stop
doing this, but allow our peers to.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Fix incompatibility with LND which prevented us opening private channels
Fixes: ElementsProject#6208
When core lightning is asking the information about
the blockchain with `getchaininfo` command lightningd
know already the information about the min and max block height.

the problem is when we have a smarter Bitcoin backend that is able
to switch between different clients in some cases is helpful
give the information about current known height by lightningd and
pass it down to the plugin.

In this way, the plugin knows what is the correct known height from lightnind, and can
try to fix some problems if any exit.

This is particularly useful when you are syncing a new backend from scratch
like https://github.com/cloudhead/nakamoto and we avoid returning the
lower height from the known, and avoid the crash of core lightning.

With this information, the plugin can start to sync the chain and return
the answer back only when the chain is in sync with the current status of
lightningd.

Another reason to add this field and not wait the correct block in core
lightning itself is because Bitcoin Core is extremely slow to sync up,
so the question here is, how long should we wait? The time depends
on various factors.

With this approach of informing the plugin about the height, in some cases,
you can start the syncing but move the execution to another backend until
the previous one is ready.

The problem I want to solve is that I don't want to be left in the dark when
we run `getchaininfo`, and I want to have the opportunity to wait for
the blockchain sync or decide to dispatch the request elsewhere.

Changelog-Added: Pass the current known block height down to the getchaininfo call.
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Ignore SHA256SUMS and SHA256SUMS.asc used for release verification
- Updated Dockerfile for amd64
- Removed linuxarm32v7.Dockerfile
- Removed linuxarm64v8.Dockerfile
- Added Dockerfile.arm32v7 for grpc enabled image
- Added Dockerfile.arm64v8 for grpc enabled image
Updating bionic, focal and jammy cl-repro builds:
- Rust version from 1.62 to 1.65
- Removed git clone, only mount option
- Removed sha256sums generation
Passing release version for make command to ensure that refresh submodule (from external makefile) runs with --force-version option as well.
Updated build release for:
- force-version and force-mtime checks
- zipfile is not optional anymore, it will be created before all other action
- CLN's amd64, arm32v7 and arm64v8 docker setup
- Verify release feature
Updated release instruction after build-release.sh fixes.
Add Docker buildx and QEMU instruction in reproducible
@ShahanaFarooqui ShahanaFarooqui closed this by deleting the head repository Aug 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet