Skip to content

ci: bump rustc to 1.60 for fuzz test#2109

Merged
apoelstra merged 4 commits intorust-bitcoin:masterfrom
vincenzopalazzo:macros/ci-fixes
Oct 7, 2023
Merged

ci: bump rustc to 1.60 for fuzz test#2109
apoelstra merged 4 commits intorust-bitcoin:masterfrom
vincenzopalazzo:macros/ci-fixes

Conversation

@vincenzopalazzo
Copy link
Copy Markdown
Contributor

Ci looks like broken, so this should fix
it

apoelstra
apoelstra previously approved these changes Oct 6, 2023
Copy link
Copy Markdown
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 6aa1b297d0dd633935636e168ea5d645c4944e08

@apoelstra
Copy link
Copy Markdown
Member

Huh. It looks like cargo had some sort of breaking change between 1.58 and 1.60.

@vincenzopalazzo
Copy link
Copy Markdown
Contributor Author

Yeah I was looking at it now

Downloaded rand_core v0.6.4
  Downloaded hex-conservative v0.1.1
  Downloaded semver v1.0.19
  Downloaded schemars_derive v0.8.12
  Downloaded itoa v1.0.9
  Downloaded rand_chacha v0.3.1
  Downloaded byteorder v1.5.0
error: failed to parse manifest at `/home/runner/.cargo/registry/src/github.com-1ecc[62](https://github.com/rust-bitcoin/rust-bitcoin/actions/runs/6436198045/job/17479072967?pr=2109#step:4:63)99db9ec823/byteorder-1.5.0/Cargo.toml`

Caused by:
  failed to parse the `edition` key

Caused by:
  this version of Cargo is older than the `2021` edition, and only supports `2015` and `2018` editions.

Then we have also

cargo 0.7.0-nightly (1af03be 2015-12-08)
+ rustc --version
rustc 1.6.0 (c30b771ad 2016-01-19)
+ cargo install --force honggfuzz --no-default-features
Unknown flag: '--force'

Usage:
    cargo install [options] [<crate>]
    cargo install [options] --list
Error: Process completed with exit code 1.

i will fix now

@vincenzopalazzo
Copy link
Copy Markdown
Contributor Author

vincenzopalazzo commented Oct 6, 2023

Trying to reproduce it on my machine

+ targetFiles=fuzz_targets/bitcoin_outpoint_string.rs
+ cargo --version
cargo 1.60.0 (d1fd9fe 2022-03-01)
+ rustc --version
rustc 1.60.0 (7737e0b5c 2022-04-04)
+ cargo install --force honggfuzz --no-default-features
    Updating crates.io index
       Fetch [==========>              ]  46.50%, (13551/48045) resolving deltas

it works, looks like the CI gets the wrong cargo version cargo 0.7.0-nightly.

Ah ofcs maybe the CI round 1.60 to 1.6, so pushing the version with 1.60.0

Can you give another shot @apoelstra the ./contrib/test.sh is running on my system now so 1.60.0 should work if get the correct cargo version.

@apoelstra
Copy link
Copy Markdown
Member

Ah ofcs maybe the CI round 1.60 to 1.6, so pushing the version with 1.60.0

Lol. Great catch!

@apoelstra
Copy link
Copy Markdown
Member

Ugh. Let's start trying random versions then. 1.64? 1.70?

@vincenzopalazzo
Copy link
Copy Markdown
Contributor Author

Mh this looks another kind of error, for example the fuzzer is a link error https://github.com/rust-bitcoin/rust-bitcoin/actions/runs/6436379883/job/17480345960?pr=2109

Then there is this, but this is not modified by me. So I will look more into this tomorrow

Run ./contrib/test.sh
+ CRATES=bitcoin hashes internals fuzz
+ DEPS=recent minimal
+ MSRV=1\.48\.0
+ cargo --version
+ grep 1\.48\.0
cargo 1.48.0 (65cbdd2dc 2020-10-14)
+ cargo update -p serde_json --precise 1.0.[9](https://github.com/rust-bitcoin/rust-bitcoin/actions/runs/6436379886/job/17480346840?pr=2109#step:4:10)9
    Updating crates.io index
    Updating git repository `https://github.com/llogiq/mutagen`
    Updating serde_json v1.0.[10](https://github.com/rust-bitcoin/rust-bitcoin/actions/runs/6436379886/job/17480346840?pr=2109#step:4:11)7 -> v1.0.99
+ cargo update -p serde --precise 1.0.156
    Updating crates.io index
    Updating serde v1.0.188 -> v1.0.156
    Updating serde_derive v1.0.188 -> v1.0.156
    Removing syn v2.0.38
+ cargo update -p quote --precise 1.0.30
    Updating crates.io index
    Updating quote v1.0.33 -> v1.0.30
+ cargo update -p proc-macro2 --precise 1.0.63
    Updating crates.io index
    Updating proc-macro2 v1.0.68 -> v1.0.63
+ cargo update -p serde_test --precise 1.0.175
    Updating crates.io index
    Updating serde_test v1.0.176 -> v1.0.175
+ cargo update -p schemars --precise 0.8.[12](https://github.com/rust-bitcoin/rust-bitcoin/actions/runs/6436379886/job/17480346840?pr=2109#step:4:13)
    Updating crates.io index
    Updating schemars v0.8.[15](https://github.com/rust-bitcoin/rust-bitcoin/actions/runs/6436379886/job/17480346840?pr=2109#step:4:16) -> v0.8.12
    Updating schemars_derive v0.8.15 -> v0.8.12
+ cargo update -p schemars_derive --precise 0.8.12
    Updating crates.io index
+ cargo update -p memchr --precise 2.5.0
    Updating crates.io index
    Updating memchr v2.6.4 -> v2.5.0
+ cargo update -p bitcoin:0.30.1 --precise 0.30.0
    Updating crates.io index
    Removing bech32 v0.9.1
    Removing bitcoin v0.30.1
+ cargo check --all-features --all-targets
 Downloading crates ...
  Downloaded itoa v1.0.9
  Downloaded rustc_version v0.4.0
  Downloaded schemars_derive v0.8.12
  Downloaded ryu v1.0.15
  Downloaded semver v1.0.19
  Downloaded hex-conservative v0.1.1
  Downloaded serde v1.0.156
  Downloaded cc v1.0.83
  Downloaded schemars v0.8.12
  Downloaded core2 v0.3.3
  Downloaded bitcoin_hashes v0.12.0
  Downloaded hex_lit v0.1.1
  Downloaded cfg-if v1.0.0
  Downloaded secp256k1 v0.27.0
  Downloaded rand_core v0.6.4
  Downloaded serde_derive_internals v0.26.0
  Downloaded serde_cbor v0.9.0
  Downloaded ppv-lite86 v0.2.[17](https://github.com/rust-bitcoin/rust-bitcoin/actions/runs/6436379886/job/17480346840?pr=2109#step:4:18)
  Downloaded byteorder v1.5.0
error: failed to parse manifest at `/home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/byteorder-1.5.0/Cargo.toml`

Caused by:
  failed to parse the `edition` key

Caused by:
  this version of Cargo is older than the `2021` edition, and only supports `2015` and `20[18](https://github.com/rust-bitcoin/rust-bitcoin/actions/runs/6436379886/job/17480346840?pr=2109#step:4:19)` editions.

vincenzopalazzo and others added 2 commits October 7, 2023 11:19
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Clippy warns: redundant guard

As suggested, remove the redundant guard.
error[E0308]: mismatched types
  --> bitcoin/src/psbt/raw.rs:87:24
   |
87 |               return Err(encode::Error::OversizedVectorAllocation {
   |  ________________________^
88 | |                 requested: key_byte_size as usize,
89 | |                 max: MAX_VEC_SIZE,
90 | |             });
   | |_____________^ expected enum `psbt::error::Error`, found enum `consensus::encode::Error`
   |
help: try wrapping the expression in `psbt::error::Error::ConsensusEncoding`
   |
87 ~             return Err(psbt::error::Error::ConsensusEncoding(encode::Error::OversizedVectorAllocation {
88 |                 requested: key_byte_size as usize,
89 |                 max: MAX_VEC_SIZE,
90 ~             }));
   |

----

  Compiling bitcoin v0.30.0 (/home/vincent/github/work/rust-btc/rust-bitcoin/bitcoin)
    Checking bitcoin-fuzz v0.0.1 (/home/vincent/github/work/rust-btc/rust-bitcoin/fuzz)
error: redundant clone
   --> bitcoin/examples/taproot-psbt.rs:453:77
    |
453 |             witness_utxo: { Some(TxOut { value, script_pubkey: script_pubkey.clone() }) },
    |                                                                             ^^^^^^^^ help: remove this
    |
    = note: `-D clippy::redundant-clone` implied by `-D warnings`
note: this value is dropped without further use
   --> bitcoin/examples/taproot-psbt.rs:453:64
    |
453 |             witness_utxo: { Some(TxOut { value, script_pubkey: script_pubkey.clone() }) },
    |                                                                ^^^^^^^^^^^^^
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone

error: could not compile `bitcoin` due to previous error
warning: build failed, waiting for other jobs to finish...
error: redundant clone
    --> bitcoin/src/psbt/mod.rs:1095:13
     |
1095 |             .clone()
     |             ^^^^^^^^ help: remove this
     |
     = note: `-D clippy::redundant-clone` implied by `-D warnings`
note: this value is dropped without further use
    --> bitcoin/src/psbt/mod.rs:1094:17
     |
1094 |           assert!(psbt
     |  _________________^
1095 | |             .clone()
     | |____________^
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone

Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
@vincenzopalazzo
Copy link
Copy Markdown
Contributor Author

OK, I see the light with the CI :) see my self-pr for the result vincenzopalazzo#2

I had also some more clippy fixes in 98513ef

Now with the new rust version, there is a fuzz test that it is failing, but I have no experience with the fuzzing test, so there is someone who can give me some tips?

The failure is here https://github.com/vincenzopalazzo/rust-bitcoin/actions/runs/6440404417/job/17489177949?pr=2

Then, I had to fix the version of the byteorder to 1.4.3 due to the recent update https://crates.io/crates/byteorder/versions

@apoelstra
Copy link
Copy Markdown
Member

Thanks @vincenzopalazzo! See #2111 -- the fuzzing issue was fixed in rust-bech32 but hasn't been released and propagated to rust-bitcoin yet.

Copy link
Copy Markdown
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK cf313ae6a1ab08555147a0f5a04faf814145d7c8

@vincenzopalazzo
Copy link
Copy Markdown
Contributor Author

Nice @apoelstra somedays I will get someone from rust-bitcoin in as room to teach me how do fuzz testing haha

@apoelstra
Copy link
Copy Markdown
Member

Hah. And thanks a ton for keeping on this! It looks like CI is passing so far. I will merge this once we get all the green checkmarks in.

@apoelstra
Copy link
Copy Markdown
Member

Ok, this looks like a pass to me, other than some things that appear to be Github bugs (complete jobs still marked as "in progress") and the deserialize_address thing which we can address sepaarately.

Copy link
Copy Markdown
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK cf313ae6a1ab08555147a0f5a04faf814145d7c8

Copy link
Copy Markdown
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 6b5d06f

@apoelstra apoelstra merged commit 342a35b into rust-bitcoin:master Oct 7, 2023
apoelstra added a commit that referenced this pull request Oct 8, 2023
3e6021b ci: fuzz test YAML toolchain future-proof (Einherjar)

Pull request description:

  To avoid YAML automatic parsing of inputs as floats, e.g. [`1.60` being parsed as `1.6`](#2109 (comment)), it is best to future-proof the `toolchain` input in CI fuzz testing as a string.

ACKs for top commit:
  vincenzopalazzo:
    ACK 3e6021b
  apoelstra:
    ACK 3e6021b
  tcharding:
    ACK 3e6021b

Tree-SHA512: 838afb401fcfbb00717d6d3efff013cda08dabe4962ee0e914c3cf438a5cc8e83fa336dbd920b299fe95f935c0c1d76cf15d3f6fc6f763ea15a1746e7254d94f
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.

4 participants