Commit Graph

2631 Commits

Author SHA1 Message Date
Deirdre Connolly 584f35ce68 Update test job to use the updated state 2021-06-04 15:14:11 -04:00
teor c453fbf6f6
Add final sapling root test vectors (#2243)
* Add sapling final root test vectors

Also tidy some formatting and imports

* Doc: final sapling roots can be duplicated

* Reverse the byte order of final sapling root test vectors

This makes the test vectors match the byte order in the block header,
rather than the zcashd RPC responses.

* Ignore pre-sapling block header commitments

Previously, Zebra expected this reserved field to be all zeroes,
but some mainnet and testnet blocks had other values.

* Test structural and semantic validation of the block commitment field

History roots are excluded from these tests, because they require
contextual validation.
2021-06-04 10:31:47 -03:00
teor 7b33278708
Update docs for coinbase maturity for NU5 (#2248) 2021-06-04 09:57:41 -03:00
teor b18c32f30f
Add the database format to the panic metadata (#2249)
Seems like it might be useful as we add more stuff to the state.
2021-06-04 14:42:15 +10:00
teor 3d2fb1c13e
Update GitHub and RFC templates based on retrospectives (#2242)
Updates:
- GitHub Issue templates
- GitHub PR templates
- RFC template

Focusing on:
- consensus rule / network reference sections
- design sections
- review/test checklist

Process changes:
- add new team members to RFC approval
- change RFC approval to "most of the team"

And general cleanup:
- delete docs from the checklist, because we now `warn(missing_docs)`
- shorter explanations
- consistent headings
- consistent order
- consistent formatting
2021-06-04 14:40:53 +10:00
Janito Vaqueiro Ferreira Filho b44d81669f
Move the check in `transaction::check::sapling_balances_match` to `V4` deserialization (#2234)
* Implement `PartialEq<i64>` for `Amount`

Allows to compare an `Amount` instance directly to an integer.

* Add `SerializationError::BadTransactionBalance`

Error variant representing deserialization of a transaction that doesn't
conform to the Sapling consensus rule where the balance MUST be zero if
there aren't any shielded spends and outputs.

* Validate consensus rule when deserializing

Return an error if the deserialized V4 transaction has a non-zero value
balance but doesn't have any Sapling shielded spends nor outputs.

* Add consensus rule link to field documentation

Describe how the consensus rule is validated structurally by
`ShieldedData`.

* Clarify that `value_balance` is zero

Make the description more concise and objective.

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>

* Update field documentation

Include information about how the consensus rule is guaranteed during
serialization.

Co-authored-by: teor <teor@riseup.net>

* Remove `check::sapling_balances_match` function

The check is redundant because the respective consensus rule is
validated structurally by `ShieldedData`.

* Test deserialization of invalid V4 transaction

A transaction with no Sapling shielded spends and no outputs but with a
non-zero balance value should fail to deserialize.

* Change least-significant byte of the value balance

State how the byte index is calculated, and change the least
significant-byte to be non-zero.

Co-authored-by: teor <teor@riseup.net>
2021-06-04 08:53:00 +10:00
teor 2f0f379a9e
Standardise clippy lints and require docs (#2238)
* Standardise lints across Zebra crates, and add missing docs

The only remaining module with missing docs is `zebra_test::command`

* Todo -> TODO

* Clarify what a transcript ErrorChecker does

Also change `Error` -> `BoxError`

* TransError -> ExpectedTranscriptError

* Output Descriptions -> Output descriptions
2021-06-04 08:48:40 +10:00
Janito Vaqueiro Ferreira Filho 9416b5d5cd
Update `transaction::check::coinbase_tx_no_joinsplit_or_spend` to validate V5 coinbase transactions with Orchard shielded data (#2236)
* Add a `Transaction::orchard_shielded_data` getter

Allows accessing the Orchard shielded data if it is present in the
transaction, regardless of the transaction version.

* Refactor `orchard_nullifiers` to use new getter

Allows making the method more concise.

* Add `CoinbaseHasEnableSpendsOrchard` error variant

Used when the validation rule is not met.

* Implement `enableSpendsOrchard` in coinbase check

The flag must not be set for the coinbase transaction.

* Refactor `Transaction::orchard_*` getters

Use the fact that `Option<T>` implements `Iterator<T>` to simplify the
code and remove the need for boxing the iterators.

Co-authored-by: teor <teor@riseup.net>
2021-06-03 01:54:08 +00:00
Alfredo Garcia a9fe0d9d3e
Make sure the mandatory checkpoint includes Canopy activation (#2235)
* Make sure the Canopy activation block is a finalized checkpoint block

This enables ZIP-221 chain history from Canopy activation onwards.

* Clarify that the mandatory checkpoint test includes Canopy activation

The test was correct, but the docs and assertion message did not include activation.

* Document that the mandatory checkpoint includes Canopy activation

Co-authored-by: teor <teor@riseup.net>
2021-06-03 10:24:08 +10:00
teor 81f2df9f36
Adjust the benchmark sample size so all benchmarks finish successfully (#2237)
This works on my machine.
2021-06-03 09:35:42 +10:00
teor 35f097995b
Make debugging easier on proptests with large vectors (#2232)
* Restore SummaryDebug on arbitrary chains

And also add it to some more proptest vectors.

* Reduce most arbitrary vectors from 10 to 4

This makes debugging easier

* Make SummaryDebug generic over collections and exact size iterators

* Document DisplayToDebug
2021-06-02 10:18:04 -03:00
Janito Vaqueiro Ferreira Filho db0cdb74ff
Update `has_inputs_and_outputs` to check V5 transactions (#2229)
* Fix documentation comment

Was missing a slash to become documentation.

* Add documentation link to type reference

Just to help navigation a bit.

* Implement `Transaction::orchard_actions()` getter

Returns an iterator to iterator over the actions in the Orchard shielded
data (if there is one, otherwise it returns an empty iterator).

* Add V5 support for `has_inputs_and_outputs`

Checks if the transaction has Orchard actions. If it does, it is
considered to have inputs and outputs.

* Refactor transaction test vectors

Make it easier to reuse the fake V5 transaction converter in other test
vectors.

* Move helper function to `zebra-chain` crate

Place it together with some other helper functions, including the one
that actually creates the fake V5 transaction.

* Test transaction with no inputs

`check::has_inputs_and_outputs` should return an error indicating that
the transaction has no inputs.

* Test transaction with no outputs

`check::has_inputs_and_outputs` should return an error indicating that
the transaction has no outputs.

* Note that transaction is fake in `expect` message

Should make the message easier to find, and also gives emphasis to the
fact that the transaction is a fake conversion to V5.

Co-authored-by: teor <teor@riseup.net>

Co-authored-by: teor <teor@riseup.net>
2021-06-02 11:32:52 +10:00
Alfredo Garcia 1685611592
Store orchard nullifiers into the state (#2185)
* add nullifier methods to orchard
* store orchard nullifiers
* bump database version
* update `IntoDisk`
* support V5 in `UpdateWith`
* add a test for finalized state
* Use the latest network upgrade in state proptests
2021-06-01 17:53:13 +10:00
teor ce45198c17
Fix comment typo: overflow -> underflow 2021-06-01 16:44:45 +10:00
teor 34702f22b6 clippy: remove needless clone and collect 2021-06-01 03:42:08 -03:00
Janito Vaqueiro Ferreira Filho 83ac1519e9 Add proptest for future `last_seen` correction
Given a generated list of gossiped peers, ensure that after running the
`validate_addrs` function none of the resulting peers have a `last_seen`
time that's after the specified limit.
2021-06-01 03:42:08 -03:00
Janito Vaqueiro Ferreira Filho 63672a2633 Test underflow handling
If the calculation to apply the compensation offset overflows or
underflows, the reported times are too distant apart, and could be sent
on purpose by a malicious peer, so all addresses from that peer should
be rejected.
2021-06-01 03:42:08 -03:00
Janito Vaqueiro Ferreira Filho f263d85aa4 Test `last_seen` time being equal to the limit
If the most recent `last_seen` time reported by a peer is exactly the
limit, the offset doesn't need to be applied because no times are in the
future.
2021-06-01 03:42:08 -03:00
Janito Vaqueiro Ferreira Filho f4a7026aa3 Test that offset is applied to all gossiped peers
Use some mock gossiped peers where some have `last_seen` times in the
past and some have times in the future. Check that all the peers have
an offset applied to them by the `validate_addrs` function.

This tests if the offset is applied to all peers that a malicious peer
gossiped to us.
2021-06-01 03:42:08 -03:00
Janito Vaqueiro Ferreira Filho 60f660e53f Test if validation doesn't offset past times
Use some mock gossiped peers that all have `last_seen` times in the
past and check that they don't have any changes to the `last_seen` times
applied by the `validate_addrs` function.
2021-06-01 03:42:08 -03:00
Janito Vaqueiro Ferreira Filho 3c9c920bbd Test if validation offsets times in the future
Use some mock gossiped peers that all have `last_seen` times in the
future and check that they all have a specific offset applied to them.
2021-06-01 03:42:08 -03:00
Janito Vaqueiro Ferreira Filho 82452621e0 Remove empty list of peers check
The `limit_last_seen_times` can now safely handle an empty list.
2021-06-01 03:42:08 -03:00
Janito Vaqueiro Ferreira Filho 966430d400 Update security note to be broader
Focus on what can go wrong, and not on the specific causes.

Co-authored-by: teor <teor@riseup.net>
2021-06-01 03:42:08 -03:00
Janito Vaqueiro Ferreira Filho f3419b7baf Handle overflow when applying offset
If an overflow occurs, the reported `last_seen` times are either very
wrong or malicious, so reject all addresses gossiped by that peer.
2021-06-01 03:42:08 -03:00
Janito Vaqueiro Ferreira Filho 5b8f33390c Add comment to describe purpose
Make it clear why all peers have the time offset applied to them.

Co-authored-by: teor <teor@riseup.net>
2021-06-01 03:42:08 -03:00
Janito Vaqueiro Ferreira Filho 9eac43a8bb Apply offset to all times received from a peer
If any of the times gossiped by a peer are in the future, apply the
necessary offset to all the times gossiped by that peer. This ensures
that all gossiped peers from a malicious peer are moved further back in
the queue.

Co-authored-by: teor <teor@riseup.net>
2021-06-01 03:42:08 -03:00
Janito Vaqueiro Ferreira Filho fa35c9b4f1 Only apply offset to times in the future
Times in the past don't have any security implications, so there's no
point in trying to apply the offset to them as well.
2021-06-01 03:42:08 -03:00
Janito Vaqueiro Ferreira Filho 876d515dd6 Improve documentation
- Make the security impact clearer and in a separate section.
- Instead of listing an assumption as almost a side-note, describe it
  clearly inside a `Panics` section.

Co-authored-by: teor <teor@riseup.net>
2021-06-01 03:42:08 -03:00
Janito Vaqueiro Ferreira Filho 54809a1b89 Don't trust reported peer `last_seen` times
Due to clock skew, the peers could end up at the front of the
reconnection queue or far at the back. The solution to this is to offset
the reported times by the difference between the most recent reported
sight (in the remote clock) and the current time (in the local clock).
2021-06-01 03:42:08 -03:00
Janito Vaqueiro Ferreira Filho 29c51d5086 Implement `MetaAddr::set_last_seen` setter method
Will be used when limiting the reported last seen times for recived
gossiped addresses.
2021-06-01 03:42:08 -03:00
Janito Vaqueiro Ferreira Filho 14ecc79f01 Use `DateTime32` in `validate_addrs` 2021-06-01 03:42:08 -03:00
Janito Vaqueiro Ferreira Filho b891a96a6d Improve ergonomics by returning `impl Iterator`
Returning `impl IntoIterator` means that the caller will always be
forced to call `.into_iter()`, and returning `impl Iterator` still
allows them to call `.into_iter()` because it becomes the identity
function.
2021-06-01 03:42:08 -03:00
Deirdre Connolly 4e8efd0cec
Split out parsing & validation of test VerificationKey from checking of the signature (#2228)
Now that we always generate an extra proper VerificationKey for each
Tweak::ChangePubkey case, this /should/ never fail: it also helps split out the
actual verification of the signature from the parsing and validation of the key
itself.
2021-06-01 15:59:34 +10:00
teor ebe1c9f88e
Add a DateTime32 type for 32-bit serialized times (#2210)
* Add a DateTime32 type for 32-bit serialized times
* Use DateTime32 for MetaAddr.last_seen
* Create and use a `DateTime32::now` method
2021-05-31 12:52:34 +10:00
teor a6e272bf1c
Fix a typo: BIP11 -> BIP111 (#2223) 2021-05-28 14:50:43 +02:00
teor 4c276dae64
Cleanup a few arbitrary impls (#2222) 2021-05-28 09:49:28 -03:00
teor 0b611eb770
Generate test chains that pass basic chain consistency tests (#2221)
* Set the tip height and previous hash for arbitrary genesis blocks

And cleanup the ledger strategy interface.

* Generate partial chains with correct previous block hashes

* Provide the network value from the PreparedChain strategy
2021-05-28 09:48:27 -03:00
Deirdre Connolly a5f5913d5f
Get redpallas tweak proptests working again (#2219)
Instead of creating an invalid verification key for a particular signature by tweaking its bytes,
create another verification key and when the ChangePubkey tweak is applied, just swap out the correct
SignatureCase::pk_bytes for SignatureCase::invalid_pk_bytes and check that trying to verify the signature
using that wrong key fails, as expected.

Resolves #2170
2021-05-27 21:59:14 +00:00
teor f94033df08
Make arbitrary block chains pass some genesis checks (#2208)
* Clarify the finalized state assertion that checks the genesis block

* Make arbitrary block chains pass some genesis checks

Use the genesis previous block hash for
- the first arbitrary block in each chain, and
- individual arbitrary blocks.

This setting can be adjusted by individual proptests as needed.
2021-05-27 12:41:20 -03:00
teor f0c271bcfe
Doc: shielded data always contains at least one action (#2218)
Remove an incorrect part of a comment
2021-05-27 12:06:08 -03:00
Conrado Gouvea f77441d49c
Fix scriptCode serialization and sighash test vectors (#2198)
* Fix scriptCode serialization and sighash test vectors

The scriptCode was being serialized without the compact size prefix, and the test vectors included the prefix in the script, which cancelled each other
2021-05-27 10:04:10 -03:00
teor 5cdcc5255f Proptest `MetaAddr` sanitization and serialization together 2021-05-26 18:13:35 -04:00
teor 9f8b4f836e Test round-trip serialization for gossiped `MetaAddr`s 2021-05-26 18:13:35 -04:00
teor 81630d19f2 Add service sanitization to `MetaAddr::sanitize`
This makes sure that deserialization and generated `MetaAddr`s are consistent.
2021-05-26 18:13:35 -04:00
teor bf6fe175dd Stop deriving PartialEq for MetaAddr
This makes sure Ord and ParitalEq are always consistent.
2021-05-26 18:13:35 -04:00
teor 078385ae00 Canonicalise arbitrary IP addresses in proptests
This makes round-trip serialization tests work.
2021-05-26 18:13:35 -04:00
teor 6fb94baeb9 Stop converting IPv6-compatible IPv4 addresses to IPv4
Zcash only uses IPv6-mapped IPv4 addresses in its network protocol.
2021-05-26 18:13:35 -04:00
teor c0114a2c5f Security: Stop panicking when serializing out-of-range times
Zebra assumes that deserialized times are always able to be serialized.

But this assumption is wrong because:
- sanitization can modify times
- gossiped `MetaAddr` validation can modify times
2021-05-26 18:13:35 -04:00
Deirdre Connolly 7894cec814
Test Eq/PartialEq for orchard keys (#2187)
* Add ConstantTimeEq's for Orchard FullViewingKey and DiversifierKey and affirmatively test

* Fix orchard::keys doc comments with links to make them automatic links

* Exercise ConstantTimeEq for FullViewingKey with a cheap clone

* Allow some clippy lints to pass for somewhat contrived tests

Co-authored-by: teor <teor@riseup.net>
2021-05-27 07:46:05 +10:00
dependabot[bot] d8fc8ac4f6 build(deps): bump inferno from 0.10.5 to 0.10.6
Bumps [inferno](https://github.com/jonhoo/inferno) from 0.10.5 to 0.10.6.
- [Release notes](https://github.com/jonhoo/inferno/releases)
- [Changelog](https://github.com/jonhoo/inferno/blob/master/CHANGELOG.md)
- [Commits](https://github.com/jonhoo/inferno/compare/v0.10.5...v0.10.6)

Signed-off-by: dependabot[bot] <support@github.com>
2021-05-26 09:57:10 -04:00