Implement sighash #870
Merged
Implement sighash #870
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
teor2345
suggested changes
Aug 11, 2020
Co-authored-by: teor <teor@riseup.net>
yaahc
added
A-rust
27 tasks
dconnolly
reviewed
Aug 13, 2020
Closed
hdevalence
reviewed
Aug 17, 2020
hdevalence
reviewed
Aug 17, 2020
hdevalence
reviewed
Aug 17, 2020
hdevalence
reviewed
Aug 17, 2020
hdevalence
reviewed
Aug 17, 2020
hdevalence
reviewed
Aug 17, 2020
Co-authored-by: teor <teor@riseup.net>
dconnolly
reviewed
Aug 26, 2020
yaahc
commented
Aug 26, 2020
Comment on lines
+162
to
+165
| /// The `input` argument indicates the transparent Input for which we are | ||
| /// producing a sighash. It is comprised of the index identifying the | ||
| /// transparent::Input within the transaction and the transparent::Output | ||
| /// representing the UTXO being spent by that input. |
There was a problem hiding this comment.
not positive this description is correct
dconnolly
reviewed
Aug 26, 2020
Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>
dconnolly
reviewed
Sep 5, 2020
dconnolly
reviewed
Sep 5, 2020
dconnolly
previously approved these changes
Sep 5, 2020
dconnolly
approved these changes
Sep 5, 2020
|
@yaahc I can merge this by squashing, but a rebase has conflicts |
|
squash is preferred, tyty |
yaahc
added a commit
that referenced
this pull request
Sep 10, 2020
add test for checking that we can verify a checkpoint
reorg things a little
switch to easier to use lines api
cleanup test itself
cleanup config logic
make semantics more obvious
fix failing tests
include extra info in test child errors and increase timeout
chain: add Transaction::hash() method.
This makes Transaction and Block have a consistent API.
chain: add transaction hash test.
chain: add docs about transaction and block hashes.
chain: impl Display for {block, transaction}::Hash
Also add a Display/FromStr round-trip proptest.
chain: rename blockheaderhash reference.
Rename old references to BlockHeaderHash and BlockHeight (#1002)
* rename some references
* Apply suggestions from code review
Co-authored-by: Deirdre Connolly <durumcrustulum@gmail.com>
Co-authored-by: teor <teor@riseup.net>
Co-authored-by: Deirdre Connolly <durumcrustulum@gmail.com>
Co-authored-by: teor <teor@riseup.net>
Implement sighash (#870)
* Implement sighash
* move sighash logic to a separate module
* start filling in more of the alg
* start setting up a test case
* make the test useful
* Iter transaction inputs
* better error message for expect
* add support for zip243 sighash
* ohey first testvector is passing, yayyy
* pass the second testvector
* add last testvector
* move a use statement
* use common deserialization code for amount everywhere
* cleanup attributes
* bring in fixed preimage
* fix discrepancy with spec
* always deserialize as a signed value
* Update zebra-chain/src/transaction/sighash.rs
* update unreachable statements
* add serialization impls for nonnegative amounts
* Apply suggestions from code review
* document sighash fn
* tweek docs
* fix mistake in translation for zip243
* consistent error messages
* reorder because i like it more that way
* document more panics
* Update zebra-chain/src/amount.rs
* Add comment regarding the serialization of spend descriptions in sighash
Co-authored-by: teor <teor@riseup.net>
Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>
build(deps): bump indexmap from 1.5.2 to 1.6.0
Bumps [indexmap](https://github.com/bluss/indexmap) from 1.5.2 to 1.6.0.
- [Release notes](https://github.com/bluss/indexmap/releases)
- [Commits](indexmap-rs/indexmap@1.5.2...1.6.0)
Signed-off-by: dependabot[bot] <support@github.com>
network: add AddressBook::potentially_connected_peers().
network: add Network::default_port().
Add test for metrics and tracing endpoints (#1000)
* add metrics and tracking endpoint tests
* test endpoints more
* add change filter test for tracing
* add await to post
* separate metrics and tracing tests
* Apply suggestions from code review
Co-authored-by: teor <teor@riseup.net>
Co-authored-by: teor <teor@riseup.net>
network: fix bug in inventory advertisement handling (#1022)
* network: fix bug in inventory advertisement handling
The RFC https://zebra.zfnd.org/dev/rfcs/0003-inventory-tracking.html described
the use of a `broadcast` channel in place of an `mpsc` channel to get
ring-buffer behavior, keeping a bound on the size of the channel but dropping
old entries when the channel is full.
However, it didn't explicitly describe how this works (the `broadcast` channel
returns a `RecvError::Lagged(u64)` to inform receivers that they lost
messages), so the lag-handling wasn't implemented and I didn't notice in
review.
Instead, the ? operator bubbled the lag error all the way up from
`InventoryRegistry::poll_inventory` through `<PeerSet as Service>::poll_ready`
through various Tower wrappers to users of the peer set. The error propagation
is bad enough, because it caused client errors that shouldn't have happened,
but there's a worse interaction.
The `Service` contract distinguishes between request errors (from
`Service::call`, scoped to the request) and service errors (from
`Service::poll_ready`, scoped to the service). The `Service` contract
specifies that once a service returns an error from `poll_ready`, the service
can be assumed to be failed permanently.
I believe (but haven't tested or carefully worked through the details) that
this caused various tower middleware to report the entire peer set service as
permanently failed due to a transient inventory "error" (more of an indicator),
and I suspect that this is the cause of #1003, where all of the sync
component's requests end up failing because the peer set reported that it
failed permanently. I am able to reproduce #1003 locally before this change
and unable to reproduce it locally after this change, though I have not tested
exhaustively.
* network: add metric for dropped inventory advertisements
Co-authored-by: teor <teor@riseup.net>
Co-authored-by: teor <teor@riseup.net>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This can be merged as-is with the understanding that no other code should be using it yet, and the associated issue 306 is not complete, to unblock a reorg of the zebra-chain crate.