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

chore: unify dependencies to root cargo.toml #5333

Merged
merged 12 commits into from
Oct 24, 2024
Merged

Conversation

syan095
Copy link
Contributor

@syan095 syan095 commented Oct 16, 2024

Pull Request

Closes: PRO-1720

Checklist

Please conduct a thorough self-review before opening the PR.

  • I am confident that the code works.
  • I have written sufficient tests.
  • I have written and tested the required migrations.
  • I have updated documentation where appropriate.

Summary

Dependencies are defined in the root cargo.toml. In other cargol.tomls, use
crate = {workspace = true }

Refactored single quotes into double quotes in all cargo.toml.

Updated crates that involve code changes

  • secp256k1 0.27 -> 0.29.1 (coupled with Bitcoin 0.32.3)
  • Bitcoin to 0.32.3
  • Sha2 to 0.10.7
  • Tokio to 1.40.0
  • Subxt to 0.37

@syan095 syan095 requested review from msgmaxim, dandanlen, a team and kylezs as code owners October 16, 2024 03:12
@syan095 syan095 requested review from jerryafr and GabrielBuragev and removed request for a team October 16, 2024 03:12
Copy link

codecov bot commented Oct 16, 2024

Codecov Report

Attention: Patch coverage is 56.09756% with 18 lines in your changes missing coverage. Please review.

Project coverage is 71%. Comparing base (09f7507) to head (ff7b39c).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
engine/src/state_chain_observer/client.rs 14% 6 Missing ⚠️
engine/src/lib.rs 0% 3 Missing ⚠️
engine/src/witness/common/epoch_source.rs 0% 2 Missing ⚠️
engine/src/btc/rpc.rs 0% 1 Missing ⚠️
engine/src/dot/http_rpc.rs 0% 1 Missing ⚠️
engine/src/elections.rs 0% 1 Missing ⚠️
...tate_chain_observer/client/extrinsic_api/signed.rs 0% 1 Missing ⚠️
engine/src/witness/arb.rs 0% 1 Missing ⚠️
...d_chain_source/chunked_by_vault/monitored_items.rs 0% 1 Missing ⚠️
engine/src/witness/dot.rs 0% 1 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##            main   #5333    +/-   ##
======================================
- Coverage     71%     71%    -0%     
======================================
  Files        494     494            
  Lines      86142   86009   -133     
  Branches   86142   86009   -133     
======================================
- Hits       61164   60968   -196     
- Misses     22245   22302    +57     
- Partials    2733    2739     +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Reverted unintended changes to `workflow/*.yml`s
Formated `cargo.toml`s

* origin/main:
  fix: don't exit if we can't remove a file that already doesn't exist (#5328)
@syan095 syan095 marked this pull request as draft October 16, 2024 04:12
@syan095 syan095 marked this pull request as ready for review October 16, 2024 04:12
@syan095 syan095 requested a review from albert-llimos October 17, 2024 07:22
@dandanlen
Copy link
Collaborator

Nice work - why did you leave out the substrate dependencies?

@syan095
Copy link
Contributor Author

syan095 commented Oct 18, 2024

Nice work - why did you leave out the substrate dependencies?

Initially, I thought PolkadotSdk only list third party dependencies in the root, but upon closer inspections, it looks like they have all dependencies listed in the root, including their own sub-crates within the same repository.
I'll make the additional changes.

cargo.toml
Updated some crates to later versions for compatibility.
chainflip-node = { workspace = true }
custom-rpc = { workspace = true }
pallet-cf-account-roles = { workspace = true }
pallet-cf-environment = { workspace = true }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Strictly speaking, I think these should activate default features when it's in a std context (and to remain consistent with the previous dependency declaration).

The way it's done in the polkadot repo is:

pallet-xx.workspace = true
pallet-xx.default-fetaures = true

I think we should follow the same convention.

This is true in a few places, not just here. For example in the node, the engine, etc. Anywhere were we previously didn't explicitly set default-features = false we now need to override our workspace default and set default-features = true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right.
I've added default-features = true for al the non- no-std crates, as well as dev-dependencies.

Refactored all single quote into double quote in cargo.tomls
* origin/main:
  feat: broker can encode btc smart contract call (#5329)
  chore: localnet recreate script can use defaults (#5338)
  feat: witnessing btc smart contract swaps (#5331)
  feat: Solana CCM fallback (#5316)
  fix: scale types for pending ceremonies (#5286)
  chore: Prune historical values in Validator pallet (#5292)
  feat: expose deposit transaction hash from ingress-egress-tracker (#5320)

# Conflicts:
#	Cargo.lock
#	engine/src/witness/btc/smart_contract.rs
* origin/main:
  feat: Submit a slot number alongside nonce (#5297)
  chore: use node version from `.nvmrc` 📌 (#5336)
  chore: add engine account_info logging (#5347)
  chore: replace manual scale encoding for ts-scale (#5335)
  chore: more consistent params in Broker API (#5342)

# Conflicts:
#	engine/src/witness/sol.rs
#	state-chain/pallets/cf-elections/Cargo.toml
#	state-chain/runtime/Cargo.toml
@syan095 syan095 enabled auto-merge October 23, 2024 22:19
@syan095 syan095 added this pull request to the merge queue Oct 24, 2024
Merged via the queue into main with commit 106de29 Oct 24, 2024
48 of 49 checks passed
@syan095 syan095 deleted the chore/unify-dependencies branch October 24, 2024 06:14
syan095 added a commit that referenced this pull request Oct 29, 2024
…waps-close-accounts

* origin/main: (44 commits)
  fix: expire all previous epochs (#5279)
  feat: add/update contract swaps parameters (#5343)
  chore: add address to solana logging (#5353)
  fix: ignore dust underflows in order fills rpc (#5352)
  chore: consistent naming prewitnessed (#5351)
  feat: engine-runner verifies gpg signature of old dylib when downloaded (#5339)
  feat: tainted transaction reporting (#5310)
  bug: change_utxo not always present (#5340)
  feat: structured error return types for rpcs (#5346)
  chore: unify dependencies to root cargo.toml (#5333)
  feat: Submit a slot number alongside nonce (#5297)
  chore: use node version from `.nvmrc` 📌 (#5336)
  chore: add engine account_info logging (#5347)
  chore: replace manual scale encoding for ts-scale (#5335)
  chore: more consistent params in Broker API (#5342)
  feat: broker can encode btc smart contract call (#5329)
  chore: localnet recreate script can use defaults (#5338)
  feat: witnessing btc smart contract swaps (#5331)
  feat: Solana CCM fallback (#5316)
  fix: scale types for pending ceremonies (#5286)
  ...

# Conflicts:
#	Cargo.lock
#	state-chain/chains/src/sol/api.rs
#	state-chain/pallets/cf-broadcast/src/migrations.rs
#	state-chain/pallets/cf-environment/Cargo.toml
dandanlen pushed a commit that referenced this pull request Oct 30, 2024
* WIP: unified some dependencies to the root cargo.toml

* Unified the rest of of dependencies into the root cargo.toml

* Reverted unintended change.

* Updated out dependencies to the (or later than the) versions used by PolkadotSdk
Updated

* Centralized pokadotSdk dependencies and our own packages into the root
cargo.toml
Updated some crates to later versions for compatibility.

* Ordered everything in the root Cargo.toml alphabetically (within sections)
Formatted all the cargo.toml

* Added default-features = true for all crates that isn't no-std.
Refactored all single quote into double quote in cargo.tomls

* Fixed release build

* Fixed storage key decoding for Engine. This will fix bouncer/engine startups
dandanlen pushed a commit that referenced this pull request Oct 31, 2024
* WIP: unified some dependencies to the root cargo.toml

* Unified the rest of of dependencies into the root cargo.toml

* Reverted unintended change.

* Updated out dependencies to the (or later than the) versions used by PolkadotSdk
Updated

* Centralized pokadotSdk dependencies and our own packages into the root
cargo.toml
Updated some crates to later versions for compatibility.

* Ordered everything in the root Cargo.toml alphabetically (within sections)
Formatted all the cargo.toml

* Added default-features = true for all crates that isn't no-std.
Refactored all single quote into double quote in cargo.tomls

* Fixed release build

* Fixed storage key decoding for Engine. This will fix bouncer/engine startups
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.

3 participants