Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements and fixes for primitive types #4 #56

Merged
merged 31 commits into from
Oct 7, 2023
Merged

Conversation

dr-orlovsky
Copy link
Member

This is mostly moving things around (between modules and other crates) and reviewing that we have all APIs and data types we need

@dr-orlovsky dr-orlovsky marked this pull request as ready for review October 7, 2023 06:36
@dr-orlovsky
Copy link
Member Author

@zoedberg I have checked that all of the changes don't break RGB Core. Also, a strict type system library id used in RGB (renamed from Bitcoin into Tx) has not changed and RGB Core strict types library does not change - meaning no semantic or memory layout changes in parts of the code used in RGB consensus has happened.

I am going to merge this and release bp-consensus v0.10.9 - feel free to post-merge review it if you have time (not required, but just to understand what we have now in BP primitives library).

With the next PR I will rename bp-primitives to bp-consensus and will do a v0.10.10 release of all BP Core crates on top of which a new RGB Core v0.10.10 will be released to make sure we have all the fixes in the RGB.

@dr-orlovsky dr-orlovsky merged commit 60ac6b5 into master Oct 7, 2023
18 checks passed
@dr-orlovsky dr-orlovsky added this to the v0.10.x milestone Oct 7, 2023
@dr-orlovsky dr-orlovsky changed the title Improvements and fixes for primitive types #3 Improvements and fixes for primitive types #4 Oct 7, 2023
@zoedberg
Copy link
Contributor

zoedberg commented Oct 9, 2023

@dr-orlovsky Looks good. But I see some warnings that may be addressed:

warning: unnecessary braces around block return value
   --> primitives/src/segwit.rs:217:46
    |
217 | #[strict_type(lib = LIB_NAME_BITCOIN, dumb = {Self::new(strict_dumb!(), vec![0; 32]).unwrap()})]
    |                                              ^                                               ^
    |
    = note: `#[warn(unused_braces)]` on by default
help: remove these braces
    |
217 - #[strict_type(lib = LIB_NAME_BITCOIN, dumb = {Self::new(strict_dumb!(), vec![0; 32]).unwrap()})]
217 + #[strict_type(lib = LIB_NAME_BITCOIN, dumb = Self::new(strict_dumb!(), vec![0; 32]).unwrap())]
    |

warning: constant `MIDSTATE_TAPSIGHASH` is never used
  --> primitives/src/taproot.rs:55:7
   |
55 | const MIDSTATE_TAPSIGHASH: [u8; 10] = *b"TapSighash";
   |       ^^^^^^^^^^^^^^^^^^^
   |
note: the lint level is defined here
  --> primitives/src/taproot.rs:54:8
   |
54 | #[warn(dead_code)]
   |        ^^^^^^^^^

warning: unnecessary braces around block return value
   --> primitives/src/segwit.rs:217:46
    |
217 | #[strict_type(lib = LIB_NAME_BITCOIN, dumb = {Self::new(strict_dumb!(), vec![0; 32]).unwrap()})]
    |                                              ^                                               ^
    |
    = note: `#[warn(unused_braces)]` on by default
help: remove these braces
    |
217 - #[strict_type(lib = LIB_NAME_BITCOIN, dumb = {Self::new(strict_dumb!(), vec![0; 32]).unwrap()})]
217 + #[strict_type(lib = LIB_NAME_BITCOIN, dumb = Self::new(strict_dumb!(), vec![0; 32]).unwrap())]
    |

Also, I've noticed that in this project and also in other RGB-related projects the clippy command in the CI doesn't include the -D warnings option that makes the build job fail when encountering warnings. I think this should be set and warnings should be addressed. Do you agree?

@dr-orlovsky
Copy link
Member Author

The warnings were fixed in #60

Also, I've noticed that in this project and also in other RGB-related projects the clippy command in the CI doesn't include the -D warnings option that makes the build job fail when encountering warnings. I think this should be set and warnings should be addressed. Do you agree?

I thought clippy also differentiates errors and warnings - or does it always treat all suggestions as warnings?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants