Non-API breaking derives for error & transaction types #558
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
|
Now with rust-bitcoin/rust-bitcoinconsensus#22 merged this is ready for the review |
|
CI is failing since it requires |
|
Can you perhaps rebase and remove the dependency-bump changes? They seem unrelated. |
|
@stevenroose done |
sgeisler
approved these changes
Apr 6, 2021
There was a problem hiding this comment.
tACK 7fe3c4a
Test log:
Apr 06 16:29:52.451 INFO testinator: Installing rust toolchain 'nightly'
Apr 06 16:29:56.835 INFO testinator: Installing rust toolchain 'stable'
Apr 06 16:29:57.434 INFO testinator: Installing rust toolchain '1.29.0'
Apr 06 16:29:58.524 INFO testinator: Preparing environment for rust 1.29.0 tests (8 configurations)
Apr 06 16:29:58.524 INFO testinator: Preparing environment for rust nightly tests (9 configurations)
Apr 06 16:29:58.524 INFO testinator: Preparing environment for rust stable tests (8 configurations)
Apr 06 16:32:30.653 INFO testinator: Running rust 1.29.0 tests in /tmp/rust-bitcoin-1.29.0.mCVODhfPtdZH/rust-bitcoin
Apr 06 16:32:30.653 DEBUG testinator: Generating lock file with rust=1.29.0
Apr 06 16:32:30.658 INFO testinator: Running rust nightly tests in /tmp/rust-bitcoin-nightly.l6h15e0SZuO1/rust-bitcoin
Apr 06 16:32:30.660 INFO testinator: Running rust stable tests in /tmp/rust-bitcoin-stable.dtypNEgI1NjJ/rust-bitcoin
Apr 06 16:32:44.273 DEBUG testinator: Pinning cc to 1.0.41
Apr 06 16:32:44.425 DEBUG testinator: Pinning serde to 1.0.98
Apr 06 16:32:44.556 DEBUG testinator: Pinning serde_derive to 1.0.98
Apr 06 16:32:44.697 DEBUG testinator: Pinning byteorder to 1.3.4
Apr 06 16:33:21.107 INFO testinator: Test rust=nightly, features=[secp-recovery] succeeded!
Apr 06 16:33:30.378 INFO testinator: Test rust=stable, features=[secp-recovery] succeeded!
Apr 06 16:34:01.598 INFO testinator: Test rust=1.29.0, features=[secp-recovery] succeeded!
Apr 06 16:34:29.033 INFO testinator: Test rust=nightly, features=[use-serde] succeeded!
Apr 06 16:34:31.180 INFO testinator: Test rust=stable, features=[use-serde] succeeded!
Apr 06 16:34:51.714 INFO testinator: Test rust=nightly, features=[base64] succeeded!
Apr 06 16:34:59.879 INFO testinator: Test rust=stable, features=[base64] succeeded!
Apr 06 16:35:20.562 INFO testinator: Test rust=nightly, features=[secp-lowmemory] succeeded!
Apr 06 16:35:31.647 INFO testinator: Test rust=stable, features=[secp-lowmemory] succeeded!
Apr 06 16:35:41.099 INFO testinator: Test rust=nightly, features=[rand] succeeded!
Apr 06 16:36:08.077 INFO testinator: Test rust=nightly, features=[unstable] succeeded!
Apr 06 16:36:08.995 INFO testinator: Test rust=stable, features=[rand] succeeded!
Apr 06 16:36:11.467 INFO testinator: Test rust=1.29.0, features=[use-serde] succeeded!
Apr 06 16:36:48.337 INFO testinator: Test rust=nightly, features=[bitcoinconsensus] succeeded!
Apr 06 16:36:52.872 INFO testinator: Test rust=stable, features=[bitcoinconsensus] succeeded!
Apr 06 16:37:34.142 INFO testinator: Test rust=1.29.0, features=[base64] succeeded!
Apr 06 16:37:34.414 INFO testinator: Test rust=nightly, features=[secp-recovery,use-serde,base64,secp-lowmemory,rand,unstable,bitcoinconsensus] succeeded!
Apr 06 16:37:46.423 INFO testinator: Test rust=stable, features=[secp-recovery,use-serde,base64,secp-lowmemory,rand,bitcoinconsensus] succeeded!
Apr 06 16:37:56.139 INFO testinator: Test rust=nightly, features=[] succeeded!
Apr 06 16:38:12.423 INFO testinator: Test rust=stable, features=[] succeeded!
Apr 06 16:38:25.280 INFO testinator: Test rust=1.29.0, features=[secp-lowmemory] succeeded!
Apr 06 16:39:13.076 INFO testinator: Test rust=1.29.0, features=[rand] succeeded!
Apr 06 16:40:02.013 INFO testinator: Test rust=1.29.0, features=[bitcoinconsensus] succeeded!
Apr 06 16:40:52.505 INFO testinator: Test rust=1.29.0, features=[secp-recovery,use-serde,base64,secp-lowmemory,rand,bitcoinconsensus] succeeded!
Apr 06 16:41:33.824 INFO testinator: Test rust=1.29.0, features=[] succeeded!
Apr 06 16:41:35.901 INFO testinator: Fuzzing deserialize_script
Apr 06 16:42:42.188 INFO testinator: Successfully fuzzed deserialize_script
Apr 06 16:42:42.188 INFO testinator: Fuzzing uint128_fuzz
Apr 06 16:43:43.146 INFO testinator: Successfully fuzzed uint128_fuzz
Apr 06 16:43:43.146 INFO testinator: Fuzzing deserialize_amount
Apr 06 16:44:44.087 INFO testinator: Successfully fuzzed deserialize_amount
Apr 06 16:44:44.087 INFO testinator: Fuzzing deserialize_transaction
Apr 06 16:45:46.243 INFO testinator: Successfully fuzzed deserialize_transaction
Apr 06 16:45:46.243 INFO testinator: Fuzzing deser_net_msg
Apr 06 16:46:49.069 INFO testinator: Successfully fuzzed deser_net_msg
Apr 06 16:46:49.069 INFO testinator: Fuzzing deserialize_address
Apr 06 16:47:50.234 INFO testinator: Successfully fuzzed deserialize_address
Apr 06 16:47:50.235 INFO testinator: Fuzzing deserialize_block
Apr 06 16:48:52.221 INFO testinator: Successfully fuzzed deserialize_block
Apr 06 16:48:52.221 INFO testinator: Fuzzing outpoint_string
Apr 06 16:49:53.256 INFO testinator: Successfully fuzzed outpoint_string
Apr 06 16:49:53.256 INFO testinator: Fuzzing deserialize_psbt
Apr 06 16:50:55.248 INFO testinator: Successfully fuzzed deserialize_psbt
|
Oh, wait, what did you rebase it on? It seems to be missing #583, so CI still fails. I mistook it for a GH Actions bug, but it isn't I think. |
sanket1729
reviewed
Apr 6, 2021
There was a problem hiding this comment.
nit: The commit message is not consistent. This PR also adds derives to TxIn/TxOut/Transaction.
I also think default is missing for Transaction. We should add a default with Transaction with version 0 or version 2, locktime 0 and empty in/outs. I think this will quite handy in testing.
I am slightly leaning towards zero because it seems a reasonable default and we can auto derive it.
Well, it was merged after I did the rebase. But to me CI here shows that all tests have passed successfully |
|
@sanket1729 I'm afraid |
dr-orlovsky
changed the title
apoelstra
approved these changes
Apr 21, 2021
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 implements first part of #555 recommendations for Error types within rust-bitcoin. This PR is (a) non-API breaking & (b) can be applied before similar PRs in other repos. It will be followed by more PRs that are (1) depend on error type derives from downstream repos & (2) will be API-breaking
PS: depends on rust-bitcoin/rust-bitcoinconsensus#22 merge & new version release