Commit graph

141 commits

Author SHA1 Message Date
Luke Parker
f7e49e1f90
Update Rust nightly
Supersedes #368.

Adds exceptions for unwrap_or_default due to preference against Default's
ambiguity.
2023-09-02 01:24:09 -04:00
Luke Parker
cd4c3a6c88
Correct publication of Completed Tributary TXs 2023-09-02 00:50:54 -04:00
Luke Parker
fddc605c65
Define a proper Topic (Zone + ID)
Removes the inefficiency of recognized_topic for attempt returning None if the
topic isn't recognized.
2023-09-01 01:21:15 -04:00
Luke Parker
2ad6b38be9
Prefix root keys in coordinator with "coordinator" to prevent conflicts with tributary 2023-09-01 01:00:24 -04:00
Luke Parker
fda90e23c9
Reduce and clarify data flow in Tributary scanner 2023-09-01 00:59:10 -04:00
Luke Parker
3f3f6b2d0c
Properly route attempt around DkgConfirmer 2023-09-01 00:16:43 -04:00
Luke Parker
fa8ff62b09
Remove sender_i from DkgShares
It was a piece of duplicated data used to achieve context-less
de)serialization. This new Vec code is a bit tricker to first read, yet overall
clean and removes a potential fault.

Saves 2 bytes from DkgShares messages.
2023-09-01 00:03:56 -04:00
Luke Parker
5113ab9ec4
Move SignCompleted to Unsigned to cause de-duplication amongst honest validators 2023-08-31 23:39:36 -04:00
Luke Parker
9b7cb688ed
Have Batch contain Block and batch ID, ensuring eclipsed validators don't publish invalid shares
See prior commit message for more info.

With the plan for the batch sign ID to be just 5 bytes (potentially 4), this
does incur a +5 bytes cost compared to the ExternalBlock system *even in the
standard case*. The simplicity remains preferred at this time.
2023-08-31 23:04:39 -04:00
Luke Parker
9a5f8fc5dd
Replace ExternalBlock with Batch
The initial TODO was simply to use one ExternalBlock per all batches in the
block. This would require publishing ExternalBlock after the last batch,
requiring knowing the last batch. While we could add such a pipeline, it'd
require:

1) Initial preprocesses using a distinct message from BatchPreprocess
2) An additional message sent after all BatchPreprocess are sent

Unfortunately, both would require tweaks to the SubstrateSigner which aren't
worth the complexity compared to the solution here, at least, not at this time.

While this will cause, if a Tributary is signing a block whose total batch data
exceeds 25 kB, to use multiple transactions which could be optimized out by
'better' local data pipelining, that's an extreme edge case. Given the temporal
nature of each Tributary, it's also an acceptable edge.

This does no longer achieve synchrony over external blocks accordingly. While
signed batches have synchrony, as they embed their block hash, batches being
signed don't have cryptographic synchrony on their contents. This means
validators who are eclipsed may produce invalid shares, as they sign a
different batch. This will be introduced in a follow-up commit.
2023-08-31 23:00:25 -04:00
Luke Parker
2dc35193c9
Handle batch n+1 being signed before batch n is 2023-08-31 22:09:34 -04:00
Luke Parker
9bf24480f4
Spawn an async test per P2P message to try and resolve latency issues 2023-08-31 02:35:50 -04:00
Luke Parker
3af9dc5d6f
Tweak Heartbeat configuration so LibP2P can be expected to deliver messages within latency window 2023-08-31 01:33:52 -04:00
Luke Parker
148bc380fe
Add assert on edge case requiring a validator with 34% and a broken invariant 2023-08-31 01:08:40 -04:00
Luke Parker
8dad62f300
Fix panic when post-verifying Precommits in log
Notes an edge case which enables invalid commit production.
2023-08-31 01:02:57 -04:00
Luke Parker
1e79de87e8
Remove contention between LibP2p spawned task and consumers via channels 2023-08-30 23:31:09 -04:00
Luke Parker
2f57a69cb6
Define BLOCK_PROCESSING_TIME, LATENCY_TIME in ms
Updates Tributary values to allow 999ms for block processing (from 2s) and
1667ms for latency (up from 1s).

The intent is to resolve #365. I don't know if this will, but it increases the
chances of success and these values should be fine in prod since Tributary is a
post-execution chain (making block procesisng time minimal).

Does embed the dagger of N::block_time() panicking if the block time in ms
doesn't cleanly divide by 1000.
2023-08-30 22:58:42 -04:00
Luke Parker
493a222421
Use a timeout in case the JSON-RPC notifications have unexpected behavior 2023-08-30 17:57:33 -04:00
Luke Parker
d5a19eca8c
Add a notification system for finalizations to serai-client, use in coordinator 2023-08-30 17:25:04 -04:00
Luke Parker
285422f71a
Add a full-stack mint and burn test for Bitcoin and Monero
Fixes where ram_scanned is updated in processor. The prior version, while safe,
would redo massive amounts of work during periods of inactivity. It also hit an
undocumented invariant where get_eventuality_completions assumes new blocks,
yet redone work wouldn't have new blocks.

Modifies Monero's generate_blocks to return the hashes of the generated blocks.
2023-08-28 21:17:22 -04:00
Luke Parker
a3649b2062
Move where we trigger KeyGen to avoid a race condition
We only expect processor messages when we have the relevant Tributary. We
queued Tributary creation, yet then kicked off processor messages. We need to
wait until the Tributary is actually created to kick off processor messages.
2023-08-27 18:23:45 -04:00
Luke Parker
c9b2490ab9
Tweak tributary_test to handle a one-block variance
Prior to the previous commit, whatever async scheduling occurred caused them to
all have the same tip. Now, some are one block ahead of others. This adds
tolerance for that, as it's an acceptable variance, so long as it's solely one
block.
2023-08-27 05:28:36 -04:00
Luke Parker
2db53d5434
Use &self for handle_message and sync_block in Tributary
They used &mut self to prevent execution at the same time. This uses a lock
over the channel to achieve the same security, without requiring a lock over
the entire tributary.

This fixes post-provided Provided transactions. sync_block waited for the TX to
be provided, yet it never would as sync_block held a mutable reference over the
entire Tributary, preventing any other read/write operations of any scope.

A timeout increased (bc2f23f72b) due to this bug
not being identified has been decreased back, thankfully.

Also shims in basic support for Completed, which was the WIP before this bug
was identified.
2023-08-27 05:07:11 -04:00
Luke Parker
22f3c9e58f
Stop trying to publish a Batch if another node does 2023-08-26 21:37:21 -04:00
Luke Parker
9adefa4c2c
Add code to handle a race condition around first_preprocess 2023-08-26 21:35:43 -04:00
Luke Parker
f249e20028
Route KeyPair so Tributary can construct SignId as needed 2023-08-25 18:37:22 -04:00
Luke Parker
8c1d8a2658
Only emit Preprocesses/Shares when participating 2023-08-24 23:50:19 -04:00
Luke Parker
32df302cc4
Move recognized_id from a channel to an async lambda
Fixes a race condition. Also fixes recognizing batch IDs.
2023-08-24 21:55:59 -04:00
Luke Parker
ea8e26eca3
Use an empty key for Batch's SignId 2023-08-24 20:39:34 -04:00
Luke Parker
bccdabb53d
Use a single Substrate signer, per intentions in #227
Removes key from Update as well, since it's no longer variable.
2023-08-24 20:30:50 -04:00
Luke Parker
b91bd44476
Support multiple batches per block by the coordinator
Also corrects an assumption block hash == batch ID.
2023-08-24 19:13:18 -04:00
Luke Parker
310a09b5a4
clippy fix 2023-08-22 01:00:18 -04:00
Luke Parker
c65bb70741
Remove SlashVote per #349 2023-08-21 23:48:33 -04:00
Luke Parker
dc88b29b92
Add keep-alive timeout to coordinator
The Heartbeat was meant to serve for this, yet no Heartbeats are fired when we
don't have active tributaries.

libp2p does offer an explicit KeepAlive protocol, yet it's not recommended in
prod. While this likely has the same pit falls as LibP2p's KeepAlive protocol,
it's at least tailored to our timing.
2023-08-21 02:36:03 -04:00
akildemir
e319762c69
use half-aggregation for tm messages (#346)
* dalek 4.0

* cargo update

Moves to a version of Substrate which uses curve25519-dalek 4.0 (not a rc).
Doesn't yet update the repo to curve25519-dalek 4.0 (as a branch does) due
to the official schnorrkel using a conflicting curve25519-dalek. This would
prevent installation of frost-schnorrkel without a patch.

* use half-aggregation for tm messages

* fmt

* fix pr comments

* cargo update

Achieves three notable updates.

1) Resolves RUSTSEC-2022-0093 by updating libp2p-identity.
2) Removes 3 old rand crates via updating ed25519-dalek (a dependency of
libp2p-identity).
3) Sets serde_derive to 1.0.171 via updating to time 0.3.26 which pins at up to
1.0.171.

The last one is the most important. The former two are niceties.

serde_derive, since 1.0.171, ships a non-reproducible binary blob in what's a
complete compromise of supply chain security. This is done in order to reduce
compile times, yet also for the maintainer of serde (dtolnay) to leverage
serde's position as the 8th most downloaded crate to attempt to force changes
to the Rust build pipeline.

While dtolnay's contributions to Rust are respectable, being behind syn, quote,
and proc-macro2 (the top three crates by downloads), along with thiserror,
anyhow, async-trait, and more (I believe also being part of the Rust project),
they have unfortunately decided to refuse to listen to the community on this
issue (or even engage with counter-commentary). Given their political agenda
they seem to try to be accomplishing with force, I'd go as far as to call their
actions terroristic (as they're using the threat of the binary blob as
justification for cargo to ship 'proper' support for binary blobs).

This is arguably representative of dtolnay's past work on watt. watt was a wasm
interpreter to execute a pre-compiled proc macro. This would save the compile
time of proc macros, yet sandbox it so a full binary did not have to be run.

Unfortunately, watt (while decreasing compile times) fails to be a valid
solution to supply chain security (without massive ecosystem changes). It never
implemented reproducible builds for its wasm blobs, and a malicious wasm blob
could still fundamentally compromise a project. The only solution for an end
user to achieve a secure pipeline would be to locally build the project,
verifying the blob aligns, yet doing so would negate all advantages of the
blob.

dtolnay also seems to be giving up their role as a FOSS maintainer given that
serde no longer works in several environments. While FOSS maintainers are not
required to never implement breaking changes, the version number is still 1.0.
While FOSS maintainers are not required to follow semver, releasing a very
notable breaking change *without a new version number* in an ecosystem which
*does follow semver*, then refusing to acknowledge bugs as bugs with their work
does meet my personal definition of "not actively maintaining their existing
work". Maintenance would be to fix bugs, not introduce and ignore.

For now, serde > 1.0.171 has been banned. In the future, we may host a fork
without the blobs (yet with the patches). It may be necessary to ban all of
dtolnay's maintained crates, if they continue to force their agenda as such,
yet I hope this may be resolved within the next week or so.

Sources:

https://github.com/serde-rs/serde/issues/2538 - Binary blob discussion

This includes several reports of various workflows being broken.

https://github.com/serde-rs/serde/issues/2538#issuecomment-1682519944

dtolnay commenting that security should be resolved via Rust toolchain edits,
not via their own work being secure. This is why I say they're trying to
leverage serde in a political game.

https://github.com/serde-rs/serde/issues/2526 - Usage via git broken

dtolnay explicitly asks the submitting user if they'd be willing to advocate
for changes to Rust rather than actually fix the issue they created. This is
further political arm wrestling.

https://github.com/serde-rs/serde/issues/2530 - Usage via Bazel broken

https://github.com/serde-rs/serde/issues/2575 - Unverifiable binary blob

https://github.com/dtolnay/watt - dtolnay's prior work on precompilation

* add Rs() api to  SchnorrAggregate

* Correct serai-processor-tests to dalek 4

* fmt + deny

* Slash malevolent validators  (#294)

* add slash tx

* ignore unsigned tx replays

* verify that provided evidence is valid

* fix clippy + fmt

* move application tx handling to another module

* partially handle the tendermint txs

* fix pr comments

* support unsigned app txs

* add slash target to the votes

* enforce provided, unsigned, signed tx ordering within a block

* bug fixes

* add unit test for tendermint txs

* bug fixes

* update tests for tendermint txs

* add tx ordering test

* tidy up tx ordering test

* cargo +nightly fmt

* Misc fixes from rebasing

* Finish resolving clippy

* Remove sha3 from tendermint-machine

* Resolve a DoS in SlashEvidence's read

Also moves Evidence from Vec<Message> to (Message, Option<Message>). That
should meet all requirements while being a bit safer.

* Make lazy_static a dev-depend for tributary

* Various small tweaks

One use of sort was inefficient, sorting unsigned || signed when unsigned was
already properly sorted. Given how the unsigned TXs were given a nonce of 0, an
unstable sort may swap places with an unsigned TX and a signed TX with a nonce
of 0 (leading to a faulty block).

The extra protection added here sorts signed, then concats.

* Fix Tributary tests I broke, start review on tendermint/tx.rs

* Finish reviewing everything outside tests and empty_signature

* Remove empty_signature

empty_signature led to corrupted local state histories. Unfortunately, the API
is only sane with a signature.

We now use the actual signature, which risks creating a signature over a
malicious message if we have ever have an invariant producing malicious
messages. Prior, we only signed the message after the local machine confirmed
it was okay per the local view of consensus.

This is tolerated/preferred over a corrupt state history since production of
such messages is already an invariant. TODOs are added to make handling of this
theoretical invariant further robust.

* Remove async_sequential for tokio::test

There was no competition for resources forcing them to be run sequentially.

* Modify block order test to be statistically significant without multiple runs

* Clean tests

---------

Co-authored-by: Luke Parker <lukeparker5132@gmail.com>

* Add DSTs to Tributary TX sig_hash functions

Prevents conflicts with other systems/other parts of the Tributary.

---------

Co-authored-by: Luke Parker <lukeparker5132@gmail.com>
2023-08-21 01:22:00 -04:00
akildemir
39ce819876
Slash malevolent validators (#294)
* add slash tx

* ignore unsigned tx replays

* verify that provided evidence is valid

* fix clippy + fmt

* move application tx handling to another module

* partially handle the tendermint txs

* fix pr comments

* support unsigned app txs

* add slash target to the votes

* enforce provided, unsigned, signed tx ordering within a block

* bug fixes

* add unit test for tendermint txs

* bug fixes

* update tests for tendermint txs

* add tx ordering test

* tidy up tx ordering test

* cargo +nightly fmt

* Misc fixes from rebasing

* Finish resolving clippy

* Remove sha3 from tendermint-machine

* Resolve a DoS in SlashEvidence's read

Also moves Evidence from Vec<Message> to (Message, Option<Message>). That
should meet all requirements while being a bit safer.

* Make lazy_static a dev-depend for tributary

* Various small tweaks

One use of sort was inefficient, sorting unsigned || signed when unsigned was
already properly sorted. Given how the unsigned TXs were given a nonce of 0, an
unstable sort may swap places with an unsigned TX and a signed TX with a nonce
of 0 (leading to a faulty block).

The extra protection added here sorts signed, then concats.

* Fix Tributary tests I broke, start review on tendermint/tx.rs

* Finish reviewing everything outside tests and empty_signature

* Remove empty_signature

empty_signature led to corrupted local state histories. Unfortunately, the API
is only sane with a signature.

We now use the actual signature, which risks creating a signature over a
malicious message if we have ever have an invariant producing malicious
messages. Prior, we only signed the message after the local machine confirmed
it was okay per the local view of consensus.

This is tolerated/preferred over a corrupt state history since production of
such messages is already an invariant. TODOs are added to make handling of this
theoretical invariant further robust.

* Remove async_sequential for tokio::test

There was no competition for resources forcing them to be run sequentially.

* Modify block order test to be statistically significant without multiple runs

* Clean tests

---------

Co-authored-by: Luke Parker <lukeparker5132@gmail.com>
2023-08-21 00:28:23 -04:00
akildemir
e680eabb62
Improve batch handling (#316)
* restrict batch size to ~25kb

* add batch size check to node

* rate limit batches to 1 per serai block

* add support for multiple batches for block

* fix review comments

* Misc fixes

Doesn't yet update tests/processor until data flow is inspected.

* Move the block from SignId to ProcessorMessage::BatchPreprocesses

* Misc clean up

---------

Co-authored-by: Luke Parker <lukeparker5132@gmail.com>
2023-08-14 11:57:38 -04:00
Luke Parker
a3441a6871
Don't have publish return the 'hash' 2023-08-14 08:18:19 -04:00
Luke Parker
acc19e2817
Stop attempting to call set_keys if another validator does
This prevents this function from hanging ad-infinitum.
2023-08-14 06:53:23 -04:00
Luke Parker
5e02f936e4
Perform MuSig signing of generated keys 2023-08-14 06:08:55 -04:00
Luke Parker
6b41c91dc2
Correct TendermintMachine sleep on construction
For logging purposes, I added code to handle negative time till start. I forgot
to only sleep on positive time till start.

Should fix the recent CI failure.
2023-08-13 05:57:14 -04:00
Luke Parker
e2901cab06
Revert round-advance on TendermintMachine::new if local clock is ahead of block start
It was improperly implemented, as it assumed rounds had a constant time
interval, which they do not. It also is against the spec and was meant to
absolve us of issues with poor performance when post-starting blockchains. The
new, and much more proper, workaround for the latter is a 120-second delay
between the Substrate time and the Tributary start time.
2023-08-13 04:35:46 -04:00
Luke Parker
13a8b0afc1
Add panic-handlers which exit on any panic
By default, tokio-spawned worker panics will only kill the task, not the
program. Due to our extensive use of panicking on invariants, we should ensure
the program exits.
2023-08-13 04:30:49 -04:00
Luke Parker
7e71450dc4
Bug fixes and log statements
Also shims next nonce code with a fine-for-now piece of code which is unviable
in production, yet should survive testnet.
2023-08-13 04:03:59 -04:00
Luke Parker
ad51c123e3
Fix Tendermint bug from prior commit 2023-08-08 16:33:09 -04:00
Luke Parker
f6f945e747
Add a LibP2P instantiation to coordinator
It's largely unoptimized, and not yet exclusive to validators, yet has basic
sanity (using message content for ID instead of sender + index).

Fixes bugs as found. Notably, we used a time in milliseconds where the
Tributary expected  seconds.

Also has Tributary::new jump to the presumed round number. This reduces slashes
when starting new chains (whose times will be before the current time) and was
the only way I was able to observe successful confirmations given current
surrounding infrastructure.
2023-08-08 15:12:47 -04:00
Luke Parker
cee788eac3
Test the Coordinator emits KeyGen
Mainly just a test that the full stack is properly set up and we've hit basic
functioning for further testing.
2023-08-06 12:38:44 -04:00
Luke Parker
aab8a417db
Have the Coordinator scan the Substrate genesis block
Also adds a workflow for running tests/coordinator.
2023-08-02 12:18:50 -04:00
Luke Parker
d5c787fea2
Add initial coordinator e2e tests 2023-08-01 19:00:48 -04:00
Luke Parker
53d86e2a29
Latest clippy 2023-08-01 02:49:31 -04:00