-
Notifications
You must be signed in to change notification settings - Fork 1
Upstream PR 471 (OrchardZSA) – internal review and fixes #200
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
Changes from 150 commits
7f8a848
1328c62
e6b7762
088abc6
1420f84
0b2988a
9b43497
985d0d2
f3ebe7a
355b569
9405f80
d8f3563
5a50fb8
efbfc19
babf1da
4681cc2
cec48d7
4e47677
cbf0a3a
43d5e77
527e29a
f0b7948
563b4e5
7d3b6df
ea0fd59
9a35108
4e1c616
c77d96c
aeb9934
b4f6281
95fcf88
8e71fff
bedc732
32eee6e
02fa582
9965a6d
7ad2bac
950b806
8bc18f7
5b003f8
f49be89
aa1d895
daf6269
477f949
d4ff716
21d7273
62d4ae7
1a00c4a
ff2ac96
081513b
139ecca
7937e5b
8b0560d
2810365
a680f41
7b943e1
f38d6b9
0ee75f5
344b647
1a1f3e7
c9a8f49
f3d9459
0f56f52
149827d
c8c84aa
ac371f0
7969b9e
47f7aae
bb9e03b
dd956b8
123b609
ee89541
73218b0
50c6310
78c8efc
39b479e
6e6112c
07b3697
3ba9e5b
e2fb49d
dd69425
410037d
a7c02d2
25020f8
8a2a8de
adc6995
97cf5a3
8b89888
fe15076
9eb97f0
3d2515b
0d2f439
69f92a3
3dbdbc5
70daf8b
40e7e10
01e85a5
a95364c
b62f72a
b8d4543
e88e261
aeec27d
123a2f4
190a50c
831ca10
75c55b2
6d557af
5c09d33
c7d57b1
37101f4
57f39bf
d56c5d5
cb539d4
5f080b4
8388060
572ba6b
502c410
8509081
cf31792
0a61d36
f2411a0
9a25bd1
7e4f28c
6616893
cbcdb71
fdd69e2
1746a4b
9f44f55
f7bcf01
c1f3dcc
806878c
fab774d
58b710b
0982ff6
826dc03
76d9b7e
8784d39
cc68804
0e496fe
532b0d2
b4517e3
d18f0d5
962564b
78b6f76
cf6d3b6
abd6ecc
fc73b8e
a7de16f
2083efe
a02fdf1
c59a2a5
ca1e342
6836da8
0de4303
36b53cb
a0012b6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| # Use the latest 2.1 version of CircleCI pipeline process engine. | ||
| # See: https://circleci.com/docs/2.0/configuration-reference | ||
| version: 2.1 | ||
|
|
||
| orbs: | ||
| slack: circleci/slack@4.1 | ||
|
|
||
| # Define a job to be invoked later in a workflow. | ||
| # See: https://circleci.com/docs/2.0/configuration-reference/#jobs | ||
| jobs: | ||
| cargo-test: | ||
| # Specify the execution environment. You can specify an image from Dockerhub or use one of our Convenience Images from CircleCI's Developer Hub. | ||
| # See: https://circleci.com/docs/2.0/configuration-reference/#docker-machine-macos-windows-executor | ||
| docker: | ||
| - image: cimg/rust:1.71.0 | ||
| # Add steps to the job | ||
| # See: https://circleci.com/docs/2.0/configuration-reference/#steps | ||
| resource_class: large # specify a resource class | ||
| steps: | ||
| - checkout | ||
| - restore_cache: | ||
| keys: | ||
| - rust-cache-v1-{{ checksum "Cargo.lock" }} | ||
| - rust-cache-v1- | ||
| - run: | ||
| name: "cargo test" | ||
| command: | | ||
| sudo apt update && sudo apt-get install libfontconfig libfontconfig1-dev libfreetype6-dev; | ||
| cargo version; | ||
| cargo test --all --all-features --release; | ||
| - save_cache: | ||
| paths: | ||
| - target | ||
| - ~/.cargo/registry | ||
| - ~/.cargo/git | ||
| key: rust-cache-v1-{{ checksum "Cargo.lock" }} | ||
| - slack/notify: | ||
| event: fail | ||
| template: basic_fail_1 | ||
| - slack/notify: | ||
| event: pass | ||
| template: basic_success_1 | ||
|
|
||
|
|
||
| # Invoke jobs via workflows | ||
| # See: https://circleci.com/docs/2.0/configuration-reference/#workflows | ||
| workflows: | ||
| build-and-test: | ||
| jobs: | ||
| - cargo-test: | ||
| context: CI-Orchard-slack | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,18 +1,24 @@ | ||
| name: CI checks | ||
|
|
||
| on: [push, pull_request] | ||
| #on: [push, pull_request] | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. str4d (Link): ```suggestion
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. |
||
| on: | ||
| push: | ||
| branches: [ main ] # Only runs on push to main | ||
| pull_request: # Runs on any PR to any branch | ||
|
|
||
| jobs: | ||
| test: | ||
| name: Test on ${{ matrix.os }} | ||
| runs-on: ${{ matrix.os }} | ||
| strategy: | ||
| matrix: | ||
| os: [ubuntu-latest, windows-latest, macOS-latest] | ||
| os: [ ubuntu-latest, windows-latest ] | ||
| # os: [ubuntu-latest, windows-latest, macOS-latest] | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. str4d (Link): Revert this change; we need to be testing on
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reverted. |
||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Run tests | ||
| run: cargo test --verbose | ||
| run: cargo test --verbose --release | ||
| - name: Verify working directory is clean | ||
| run: git diff --exit-code | ||
|
|
||
|
|
@@ -111,7 +117,7 @@ jobs: | |
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Generate coverage report | ||
| run: cargo tarpaulin --engine llvm --all-features --release --timeout 600 --out xml | ||
| run: timeout --preserve-status 300s cargo tarpaulin --engine llvm --timeout 600 --out xml --skip-clean || true | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. str4d (Link): Undo this change.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. |
||
| - name: Upload coverage to Codecov | ||
| uses: codecov/codecov-action@v3.1.4 | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| /target | ||
| **/*.rs.bk | ||
| .vscode | ||
| .idea | ||
| action-circuit-layout.png |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ authors = [ | |
| "Kris Nuttycombe <kris@electriccoin.co>", | ||
| ] | ||
| edition = "2021" | ||
| rust-version = "1.70" | ||
| rust-version = "1.71" | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. str4d (Link): What 1.71 functionality made this MSRV bump necessary? If it is indeed necessary, document the MSRV bump in the changelog.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed (changed to |
||
| description = "The Orchard shielded transaction protocol" | ||
| license = "MIT OR Apache-2.0" | ||
| repository = "https://github.com/zcash/orchard" | ||
|
|
@@ -30,6 +30,7 @@ ff = { version = "0.13", default-features = false } | |
| fpe = { version = "0.6", default-features = false, features = ["alloc"] } | ||
| group = "0.13" | ||
| hex = { version = "0.4", default-features = false, features = ["alloc"] } | ||
| k256 = { version = "0.13.0", default-features = false, features = ["arithmetic", "schnorr"] } | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. str4d (Link): We use the Additionally, the dependency must be optional, behind a default-disabled feature flag, because it is only required for issuers (which will almost certainly not be all wallets), or issuance verifiers (i.e. full nodes). An Orchard-only wallet must not be required to depend on it.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed (replaced |
||
| lazy_static = "1" | ||
| memuse = { version = "0.2.2", default-features = false } | ||
| pasta_curves = "0.5" | ||
|
|
@@ -109,5 +110,7 @@ debug = true | |
| debug = true | ||
|
|
||
| [patch.crates-io] | ||
| sinsemilla = { git = "https://github.com/zcash/sinsemilla", rev = "aabb707e862bc3d7b803c77d14e5a771bcee3e8c" } | ||
| zcash_note_encryption = { git = "https://github.com/zcash/zcash_note_encryption", rev = "9f7e93d42cef839d02b9d75918117941d453f8cb" } | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. str4d (Link): I checked these revisions. |
||
| halo2_gadgets = { git = "https://github.com/zcash/halo2", rev = "2308caf68c48c02468b66cfc452dad54e355e32f" } | ||
| halo2_proofs = { git = "https://github.com/zcash/halo2", rev = "2308caf68c48c02468b66cfc452dad54e355e32f" } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,6 +1,7 @@ | ||||||
| # orchard [](https://crates.io/crates/orchard) # | ||||||
| # orchard [](https://crates.io/crates/orchard) [](https://github.com/QED-it/orchard/actions/workflows/ci.yml) | ||||||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. str4d (Link): Undo this change. |
||||||
| # | ||||||
|
|
||||||
| Requires Rust 1.66+. | ||||||
| Requires Rust 1.71+. | ||||||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. str4d (Link): If the MSRV bump is not necessary and is undone:
Suggested change
|
||||||
|
|
||||||
| ## Documentation | ||||||
|
|
||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
str4d (Link): We do not use CircleCI, and this workflow references various private qedit Slack workflows. Either remove this file from your
zsa1integration branch before the PR is merged, or open a dedicated PR for merging your ongoing work that removes it (if you want to keep this CI config around for your own testing).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed (removed CircleCI config).