Commit Graph

15 Commits

Author SHA1 Message Date
Alfredo Garcia 14882183c3
document transaction sapling types (#3501) 2022-02-11 03:16:16 +00:00
Alfredo Garcia 29ad801a35
docs: Transaction consensus rules: Size rules (#3461)
* refactor transaction size consensus rules

* quote mssing consensus rule

* nit

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

* move consensus rule doc

Co-authored-by: teor <teor@riseup.net>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
2022-02-08 20:28:40 +00:00
Deirdre Connolly 9734ea1014
Check jubjub key correctness independent of redjubjub / jubjub (#3154)
* Ensure that sapling::keys::TransmissionKey jubjub point is always in the prime order group

* Adjust TransmissionKey check; add AuthorizingKey check

* Move ValueCommitment small order check to deserialization

* Apply suggestions from code review

Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>

* Use is_torsion_free() instead of is_identity() and is_prime_order()

* Add EphemeralPublicKey small order check on instantiation; remove old checks

* Use VerificationKey instead of VerificationKeyBytes; fix tests

* Use ValidatingKey instead of VerificationKeyBytes for rk

* Reject identity when creating an Orchard EphemeralPublicKey

* Make documentation more consistent, use generator in tests

* s/JubJub/Jubjub/

* Fix zebra-consensus tests (insert_fake_orchard_shielded_data)

* Create NotSmallOrderValueCommitment, since intermediate values can be the identity

* Clarify documentation

* rustdoc

* Tidy rustdoc

Co-authored-by: Conrado Gouvea <conrado@zfnd.org>
Co-authored-by: Pili Guerra <mpguerra@users.noreply.github.com>
2021-12-22 15:10:34 +00:00
Deirdre Connolly e6ffe374d4
Validate sapling, orchard anchors (#3084)
* Add Transaction::sprout_joinsplits()

* Add Anchor variants to ValidateContextError

* Make Chain anchor collections pub(crate)

* tracing::instrument several methods in state

* Add contains_*_anchors methods to FinalizedState

* Add check::anchors module and function

* Verify that anchors_refer_to_earlier_treestates in when updating chains in non-finalized state

* Update zebra-state/src/service/check/anchors.rs

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

* Add anchors() to sapling::ShieldedData

* Add sapling_anchors() to Transaction

* Use Transaction::sapling_anchors() in the anchors_refer_to_earlier_treestates() check

* Whoops, itertools

* Add a comment for improvement

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

* Add & use a cfg(test) method on FinalizedState to prep test state with anchors to allow other tests to pass contextual checks

* Allow test nullifier checks to pass by populating anchor sets, allowing test anchor checks to pass

* Add mainnet block 419202 and its sapling note commitment tree root to test vectors

* Test sapling anchor verification using the first few Sapling blocks data

* Correct comment

* assert_eq instead of assert(bool)

Co-authored-by: Conrado Gouvea <conrado@zfnd.org>

* Update zebra-state/src/service/non_finalized_state.rs

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

Co-authored-by: teor <teor@riseup.net>
Co-authored-by: Marek <mail@marek.onl>
Co-authored-by: Conrado Gouvea <conrado@zfnd.org>
2021-11-30 16:05:35 +00:00
Dimitris Apostolou afb8b3d477
Fix typos (#3055)
Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
2021-11-12 19:30:22 +00:00
Janito Vaqueiro Ferreira Filho 9e78a8af40
Refactor mempool spend conflict checks to increase performance (#2826)
* Add `HashSet`s to help spend conflict detection

Keep track of the spent transparent outpoints and the revealed
nullifiers.

Clippy complained that the `ActiveState` had variants with large size
differences, but that was expected, so I disabled that lint on that
`enum`.

* Clear the `HashSet`s when clearing the mempool

Clear them so that they remain consistent with the set of verified
transactions.

* Use `HashSet`s to check for spend conflicts

Store new outputs into its respective `HashSet`, and abort if a
duplicate output is found.

* Remove inserted outputs when aborting

Restore the `HashSet` to its previous state.

* Remove tracked outputs when removing a transaction

Keep the mempool storage in a consistent state when a transaction is
removed.

* Remove tracked outputs when evicting from mempool

Ensure eviction also keeps the tracked outputs consistent with the
verified transactions.

* Refactor to create a `VerifiedSet` helper type

Move the code to handle the output caches into the new type. Also move
the eviction code to make things a little simpler.

* Refactor to have a single `remove` method

Centralize the code that handles the removal of a transaction to avoid
mistakes.

* Move mempool size limiting back to `Storage`

Because the evicted transactions must be added to the rejected list.

* Remove leftover `dbg!` statement

Leftover from some temporary testing code.

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

* Remove unnecessary `TODO`

It is more speculation than planning, so it doesn't add much value.

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

* Fix typo in documentation

The verb should match the subject "transactions" which is plural.

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

* Add a comment to warn about correctness

There's a subtle but important detail in the implementation that should
be made more visible to avoid mistakes in the future.

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

* Remove outdated comment

Left-over from the attempt to move the eviction into the `VerifiedSet`.

* Improve comment explaining lint removal

Rewrite the comment explaining why the Clippy lint was ignored.

* Check for spend conflicts in `VerifiedSet`

Refactor to avoid API misuse.

* Test rejected transaction rollback

Using two transactions, perform the same test adding a conflict to both
of them to check if the second inserted transaction is properly
rejected. Then remove any conflicts from the second transaction and add
it again. That should work, because if it doesn't it means that when the
second transaction was rejected it left things it shouldn't in the
cache.

* Test removal of multiple transactions

When removing multiple transactions from the mempool storage, all of the
ones requested should be removed and any other transaction should be
still be there afterwards.

* Increase mempool size to 4, so that spend conflict tests work

If the mempool size is smaller than 4,
these tests don't fail on a trivial removal bug.
Because we need a minimum number of transactions in the mempool
to trigger the bug.

Also commit a proptest seed that fails on a trivial removal bug.
(This seed fails if we remove indexes in order,
because every index past the first removes the wrong transaction.)

* Summarise transaction data in proptest error output

* Summarise spend conflict field data in proptest error output

* Summarise multiple removal field data in proptest error output

And replace the very large proptest debug output with the new summary.

Co-authored-by: teor <teor@riseup.net>
2021-10-10 23:54:46 +00:00
Janito Vaqueiro Ferreira Filho a0d45c38f3
Reject conflicting mempool transactions (#2765)
* Add `Transaction::spent_outpoints` getter method

Returns an iterator over the UTXO `OutPoint`s spent by the transaction.

* Add `mempool::Error::Conflict` variant

An error representing that a transaction was rejected because it
conflicts with another transaction that's already in the mempool.

* Reject conflicting mempool transactions

Reject including a transaction in the mempool if it spends outputs
already spent by, or reveals nullifiers already revealed by another
transaction in the mempool.

* Fix typo in documentation

Remove the `r` that was incorrectly added.

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

* Specify that the conflict is a spend conflict

Make the situation clearer, because there are other types of conflict.

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

* Clarify that the outpoints are from inputs

Because otherwise it could lead to confusion because it could also mean
the outputs of the transaction represented as `OutPoint` references.

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

* Create `storage::tests::vectors` module

Refactor to follow the convention used for other tests.

* Add an `AtLeastOne::first_mut` method

A getter to allow changing the first element.

* Add an `AtLeastOne::push` method

Allow appending elements to the collection.

* Derive `Arbitrary` for `FieldNotPresent`

This is just to make the code that generates arbitrary anchors a bit
simpler.

* Test if conflicting transactions are rejected

Generate two transactions (either V4 or V5) and insert a conflicting
spend, which can be either a transparent UTXO, or a nullifier for one of
the shielded pools. Check that any attempt to insert both transactions
causes one to be accepted and the other to be rejected.

* Delete a TODO comment that we decided not to do

Co-authored-by: teor <teor@riseup.net>
2021-09-28 01:03:08 +00:00
Deirdre Connolly e719c46b1b
Track anchors and note commitment trees in zebra-state (#2458)
* Tidy chain Cargo.toml

* Organize imports

* Add method to get note commitments from all Actions in Orchard shielded data

* Add method to get note commitments from all JoinSplits in Sprout JoinSplitData

* Add Request and Response variants for awaiting anchors

* Add anchors and note commitment trees to finalized state db

* Add (From|Into)Disk impls for tree::Roots and stubs for NoteCommitmentTrees

* Track anchors and note commitment trees in Chain

Append note commitments to their trees when doing update_chain_state_with,
then use the resulting Sapling and Orchard roots to pass to history_tree, and add
new roots to the anchor sets.

* Handle errors when appending to note commitment trees

* Add comments explaining why note commitment are not removed from the tree in revert_chain_state_with

* Implementing note commitments in finalized state

* Finish serialization of Orchard tree; remove old tree when updating finalize state

* Add serialization and finalized state updates for Sprout and Sapling trees

* Partially handle trees in non-finalized state. Use Option for trees in Chain

* Rebuild trees when forking; change finalized state tree getters to not require height

* Pass empty trees to tests; use empty trees by default in Chain

* Also rebuild anchor sets when forking

* Use empty tree as default in finalized state tree getters (for now)

* Use HashMultiSet for anchors in order to make pop_root() work correctly

* Reduce DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES and MAX_PARTIAL_CHAIN_BLOCKS

* Reduce DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES and MAX_PARTIAL_CHAIN_BLOCKS even more

* Apply suggestions from code review

* Add comments about order of note commitments and related methods/fields

* Don't use Option for trees

* Set DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES=1 and restore MAX_PARTIAL_CHAIN_BLOCKS

* Remove unneeded anchor set rebuilding in fork()

* Improve proptest formatting

* Add missing comparisons to eq_internal_state

* Renamed sprout::tree::NoteCommitmentTree::hash() to root()

* Improve comments

* Add asserts, add issues to TODOs

* Remove impl Default for Chain since it was only used by tests

* Improve documentation and assertions; add tree serialization tests

* Remove Sprout code, which will be moved to another branch

* Add todo! in Sprout tree append()

* Remove stub request, response *Anchor* handling for now

* Add test for validating Sapling note commitment tree using test blocks

* Increase database version (new columns added for note commitment trees and anchors)

* Update test to make sure the order of sapling_note_commitments() is being tested

* Improve comments and structure of the test

* Improve variable names again

* Rustfmt

Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>
Co-authored-by: Conrado P. L. Gouvea <conradoplg@gmail.com>
Co-authored-by: Conrado Gouvea <conrado@zfnd.org>
Co-authored-by: teor <teor@riseup.net>
2021-07-29 09:37:18 -04:00
Alfredo Garcia ee3c992ca6
Calculate the remaining value in the transparent transaction value pool (#2486)
* add value_balance methods to transparent and shielded

* add value_balance() to transaction

* check the remaining value consensus rule

* change error name

* fix doc and nitpick

* refactor value_balance() method for joinsplit

* changes to value_balance() of Inputs

* implement joinsplits() method(not working)

* remove created methods

* remove special case

* change return error in utilities

* move utils functions to transaction methods

* fix the docs

* simplify some code

* add constrains explicitly

* remove turbofish

* refactor some transaction methods

* fix value balance signs, add docs

* simplify some code

* avoid panic in consensus check

* add missing doc

* move remaining value balance check to the state

* make changes from the last review

Co-authored-by: teor <teor@riseup.net>
2021-07-29 13:49:36 +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 d3836e6fd8 Move spends_per_anchor to TransferData
And update its docs for ZIP-244
2021-04-21 01:32:00 -04:00
teor 53779d2a3c
Redesign Sapling data model for V5 shared anchor and spends (#2021)
* Redesign Sapling data model for V5 shared anchor and spends

The shared anchor is only present if there are any spends.

As part of this change, delete the manual PartialEq impl and its tests,
because we can derive PartialEq now.

* Stop creating a temporary Vec for the spend and output iterators

* Rename TransferData variants

Interactive rename using the following commands:
```sh
fastmod Spends SpendsAndMaybeOutputs
fastmod NoSpends JustOutputs
```

* Refactor out common sprout nullifier code

* Implement the AtLeastOne constrained vector type

This vector wrapper ensures that it always contains at least one element.

* Simplify Sapling TransferData using AtLeastOne

Also update the RFC to use AtLeastOne for Orchard.
2021-04-20 16:22:25 +10:00
Alfredo Garcia e42442d48b
Redesign Transaction V5 serialization, impl trusted vector security, nullifier utility functions (#1996)
* add sapling shielded data to transaction V5

* implement nullifiers

* test v5 in shielded_data_roundtrip

* Explicitly design serialization for Transaction V5

Implement serialization for V4 and V5 spends and outputs, to make sure
that the design works.

* Test serialization for v5 spends and outputs

Also add a few missing v4 tests.

* Delete a disabled proptest

* Make v5 transactions a top-level heading

And add a missing serialized type.

* Fix a comment typo

* v5 transaction RFC: split array serialization

Based on #2017

* RFC: explicitly describe serialized field order

And link to the spec

* RFC: add the shared anchor serialization rule test

Co-authored-by: teor <teor@riseup.net>
2021-04-16 08:19:28 +10:00
teor 418575458e
Rename the () placeholder to FieldNotPresent (#1987)
* Rename the () placeholder to FieldNotPresent

* Use a unit struct

* Update the RFC
2021-04-07 09:34:58 -03:00
Alfredo Garcia 48a8a7b851
Refactor Sapling data and use it in V4 (#1946)
* start refactoring transaction v4 for transaction v5
- move ShieldedData to sapling
- add AnchorVariant
- rename shielded_data to sapling_shielded data in V4
- move value_balance into ShieldedData
- update prop tests for new structure

* add AnchorVariant to Spend
- make anchor types available from sapling crate
- update serialize

* change shielded_balances_match() arguments
* change variable name anchor to shared_anchor in ShieldedData
* fix empty value balance serialization
* use AnchorV in shielded spends
* Rename anchor to per_spend_anchor

* Use nullifiers function directly in non-finalized state
* Use self.value_balance instead of passing it as an argument

* Add missing fields to ShieldedData PartialEq
* Derive Copy for tag types

* Add doc comments for ShieldedData refactor
* Implement a per-spend anchor compatibility iterator

Co-authored-by: teor <teor@riseup.net>
2021-04-01 07:34:25 +10:00