* Clear upons upon round, not block
* Cache the proposal for a round
* Rebase onto develop, which reverted this PR, and re-apply this PR
* Set participation upon participation instead of constantly recalculating
* Cache message instances
* Add missing txn commit
Identified by @akildemir.
* Correct clippy lint identified upon rebase
* Fix tendermint chain sync (#581)
* fix p2p Reqres protocol
* stabilize tributary chain sync
* fix pr comments
---------
Co-authored-by: akildemir <34187742+akildemir@users.noreply.github.com>
* Rewrite tendermint's message handling loop to much more clearly match the paper
No longer checks relevant branches upon messages, yet all branches upon any
state change. This is slower, yet easier to review and likely without one or
two rare edge cases.
When reviewing, please see page 5 of https://arxiv.org/pdf/1807.04938.pdf.
Lines from the specified algorithm can be found in the code by searching for
"// L".
* Sane rebroadcasting of consensus messages
Instead of broadcasting the last n messages on the Tributary side of things, we
now have the machine rebroadcast the message tape for the current block.
* Only rebroadcast messages which didn't error in some way
* Only rebroadcast our own messages for tendermint
Instead of saving, for every sent message, if it was sent or not, we track the
latest block/round participated in. These two keys are comprehensive to all
prior block/rounds. We then use three keys for the latest round's
proposal/prevote/precommit, enabling tracking current state as necessary to
prevent equivocations with just 5 keys.
The storage of the latest three messages also enables proper rebroadcasting of
the current round (not implemented in this commit).
Part of https://github.com/serai-dex/serai/issues/345.
The lack of full DB persistence does mean enough nodes rebooting at the same
time may cause a halt. This will prevent slashes.
* complete various todos
* fix pr comments
* Document bounds on unique hashes in TransactionKind
---------
Co-authored-by: Luke Parker <lukeparker5132@gmail.com>
Closes https://github.com/serai-dex/serai/issues/342.
Under ideal network conditions, this is fine. While I won't claim ideal network
conditions will occur IRL, b0fcdd3367 has the
Tributary rebroadcast messages and should brute-force its way into a
functioning system.
* add reasons to slash evidence
* fix CI failing
* Remove unnecessary clones
.encode() takes &self
* InvalidVr to InvalidValidRound
* Unrelated to this PR: Clarify reasoning/potentials behind dropping evidence
* Clarify prevotes in SlashEvidence test
* Replace use of read_to_end
* Restore decode_signed_message
---------
Co-authored-by: Luke Parker <lukeparker5132@gmail.com>
If a crate has std set, it should enable std for all dependencies in order to
let them properly select which algorithms to use. Some crates fallback to
slower/worse algorithms on no-std.
Also more aggressively sets default-features = false leading to a *10%*
reduction in the amount of crates coordinator builds.
* fix typos
* remove tributary sleeping
* handle not locally provided txs
* use topic number instead of waiting list
* Clean-up, fixes
1) Uses a single TXN in provided
2) Doesn't continue on non-local provided inside verify_block, skipping further
execution of checks
3) Upon local provision of already on-chain TX, compares
---------
Co-authored-by: Luke Parker <lukeparker5132@gmail.com>
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.
* 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>
* 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>
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.
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.
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.
We had a race condition where'd we be informed of blocks 1 .. 3, and
immediately add 1 .. 3. Because we immediately tried to add 2 after 1, it'd
fail since the tip was still the genesis, yet 2 needs the tip to be 1.
Adding a channel, while ugly, was the simplest way to accomplish this.
Also has any added block be broadcasted. Else there's a race condition where a
node which syncs up to the most recent block does so, yet fails to add the next
block when it's committed to.
This defines the tart of a very complex series of locks I'm really unhappy
with. At the same time, there's not immediately a better solution. This also
should work without issue.
Impls a LocalP2p for testing.
Moves rebroadcasting into Tendermint, since it's what knows if a message is
fully valid + original.
Removes TributarySpec::validators() HashMap, as its non-determinism caused
different instances to have different round robin schedules. It was already
prior moved to a Vec for this issue, so I'm unsure why this remnant existed.
Also renames the GH no-std workflow from the prior commit.
Step moved a step forward after an externally synced/added block. This created
a race condition to add the block between the sync process and the Tendermint
machine. Now that the block routes through Tendermint, there is no such race
condition.
