In this commit, we take advantage of the graph SQL migration and use it
to also extract DNS addresses from the opaque address type. We use
opaque addresses to store addresses that we dont understand yet. We
recently added logic for DNS addresses and so we may have persisted node
announcements that have DNS addresses but we would currently have them
stored under the opaque address type. So we use this migration to see if
we can extract such addresses.
A few decisions were made here:
1) If multiple DNS addressees are extracted, this is ok and we continue
to migrate the node even though this is actually invalid at a
protocol level. We will currently check (at a higher level) that a node
announcement only has 1 DNS address in it before we broadcast it though.
2) If an invalid DNS address is encountered (so we hit the DNS type
descriptor but then the rest of the DNS address payload is invalid
and cannot be parsed into the expected hostname:port, then we skip
migrating the node completely.
In this commit, the logic in ReadElements that is used to read a single
address descriptor from a io.Reader is separated out into a new function
so that the logic can be re-used elsewhere.
The first byte of an opaque addr must be one that we dont understand
yet. We do this update in preparation for doing an on-the-fly parse of
persisted opaque addrs to see if they contain addrs that we now support.
For this to work, the first byte cant be 0x01 since this maps to a known
address.
We may have already persisted node announcements that have multiple DNS
addresses since we may have received them before updating our code to
check for this. So here we just make sure not to send these on to our
peers.
Check that the node ann doesnt contain more than 1 DNS addr.
This will ensure that we now start rejecting new node announcements
with multiple DNS addrs since this check is called in the gossiper
before persisting a node ann to our local graph.
It also validates the DNS fields according to BOLT #7 specs.
In this commit, we remove `AddrLen` as prepration step
before adding DNS address type which will have a var length.
Co-authored-by: Elle Mouton <elle.mouton@gmail.com>
PureTLVMessage describes an LN message that is a pure TLV stream. If the
message includes a signature, it will sign all the TLV records in the
inclusive ranges: 0 to 159 and 1000000000 to 2999999999.
A comprehensive test is added that shows how two versions of the same
message remain forward compatible.
This commit simplifies insertChanEdgePolicyMig. Much of the logic can be
removed given that this method is only used in the context of the graph
SQL migration.
This should improve the performance of the migration quite a lot since
it removes the extra GetChannelAndNodesBySCID call.
Finally, we make the channel-policy part of the SQL migration idempotent
by adding a migration-only policy insert query which will not error out
if the policy already exists and does not have a timestamp that is newer
than the existing records timestamp. To keep the commit simple, a
insertChanEdgePolicyMig function is added which is basically identical
to the updateChanEdgePolicy function except for the fact that it uses
the newly added query. In the next commit, it will be simplified even
more.
In this commit, we make the channel part of the graph SQL migration
idempotent (retry-safe!). We do this by adding a migration-only channel
insert query that will not error out if a the query is called and a
chanenl with the given scid&version already exists. We also ensure that
errors are not thrown if existing channel features & extra types are
re-added.
There is no need to use the "collect-then-update" pattern for node
insertion during the SQL migration since if we do have any previously
persisted data for the node and happen to re-run the insertion for that
node, the data will be exactly the same. So we can make use of "On
conflict, no nothing" here too.
In this commit, the graph SQL migration is updated so that the node
migration step is retry-safe. This is done by using migration specific
logic & queries that do not use the same node-update-constraint as the
normal node upsert logic. For normal "run-time" logic, we always expect
a node update to have a newer timestamp than any previously stored one.
But for the migration, we will only ever be dealing with a single
announcement for a given node & to make things retry-safe, we dont want
the query to error if we re-insert the exact same node.
In preparation for handling retries on the source DB side, we thread
through the `reset` call-backs properly so that we can reset appropriate
variables.
In preparation for making the channel & policy migration logic
idempotent in a step-by-step manner, we add a test here that only tests
the migration of channels _without_ policies so that we can first focus
on just making the channel migration idempotent.
Currently, the graph SQL migration is not retry safe. Meaning that if
the source DB exeutes a retry under the hood, this could result in the
migration failing. In preparation for fixing this, we adust the
migration test accordingly.
This will help us test idempotency later on, but it also ensures that
TestMigrateGraphToSQL is properly testing writes to the
graph_channel_policy_extra_types table.
Use the new feature of Go 1.24, fix linter warnings.
This change was produced by:
- running golangci-lint run --fix
- sed 's/context.Background/t.Context/' -i `git grep -l context.Background | grep test.go`
- manually fixing broken tests
- itest, lntest: use ht.Context() where ht or hn is available
- in HarnessNode.Stop() we keep using context.Background(), because it is
called from a cleanup handler in which t.Context() is canceled already.
