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

fix: ckb-hash feature ckb-contract is invalid #4122

Merged
merged 2 commits into from
Aug 30, 2023

Conversation

EthanYuan
Copy link
Collaborator

@EthanYuan EthanYuan commented Aug 28, 2023

What problem does this PR solve?

Problem Summary:

Due to a Rust issue, the features = "ckb-contract" in ckb-hash crate Cargo.toml below did not actually work as expected:

[target.'cfg(not(any(target_arch = "wasm32", features = "ckb-contract")))'.dependencies]
blake2b = { package = "blake2b-rs", version = "0.2" }

[target.'cfg(any(target_arch = "wasm32", features = "ckb-contract"))'.dependencies]
blake2b = { package = "blake2b-ref", version = "0.2.0" }

This will result in the following two CI failures of the PR refactor: Replace ckb-standalone-types with ckb-gen-types for the ckb-std CI:

cargo build --target=riscv64imac-unknown-none-elf --no-default-features --features=ckb-types,allocator
cargo build --verbose --target=riscv64imac-unknown-none-elf --features=build-with-clang

What is changed and how it works?

What's Changed:

  1. Implemented a workaround to bypass the issue
[features]
default = ["blake2b"]
ckb-contract = ["blake2b-ref"] # This feature is used for CKB contract development

[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
blake2b = { package = "blake2b-rs", version = "0.2", optional = true }

[target.'cfg(target_arch = "wasm32")'.dependencies]
blake2b = { package = "blake2b-ref", version = "0.3", optional = true }

[dependencies]
blake2b-ref = { version = "0.3", optional = true }
  1. update blake2b-ref version to 0.3.

Related changes

  • PR to update owner/repo:
  • Need to cherry-pick to the release branch

Check List

Tests

Manual test: make the CI tests of ckb-std pass:

Release note

None: Exclude this PR from the release note.

@EthanYuan EthanYuan requested a review from a team as a code owner August 28, 2023 12:31
@EthanYuan EthanYuan requested review from doitian and removed request for a team August 28, 2023 12:31
@doitian doitian added this pull request to the merge queue Aug 29, 2023
@EthanYuan EthanYuan closed this Aug 29, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a manual request Aug 29, 2023
@EthanYuan EthanYuan deleted the fix-gen-types-features branch August 29, 2023 08:54
@EthanYuan EthanYuan restored the fix-gen-types-features branch August 29, 2023 08:55
@EthanYuan EthanYuan reopened this Aug 29, 2023
@EthanYuan
Copy link
Collaborator Author

I accidentally deleted the branch before merging, and I have now restored the branch. @doitian

@doitian doitian added this pull request to the merge queue Aug 30, 2023
Merged via the queue into nervosnetwork:develop with commit 9cf92d8 Aug 30, 2023
68 checks passed
@EthanYuan EthanYuan mentioned this pull request Sep 1, 2023
doitian added a commit that referenced this pull request Sep 7, 2023
@EthanYuan EthanYuan deleted the fix-gen-types-features branch September 11, 2023 02:05
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