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

bump(zcash_script): Bump zcash script v0.1.15 and restore Windows support #8393

Merged
merged 14 commits into from
Apr 22, 2024

Conversation

oxarbitrage
Copy link
Contributor

@oxarbitrage oxarbitrage commented Apr 11, 2024

Motivation

Updates zcash_script to 0.1.15 and zcash_primitives to 0.13.0.

PR Author Checklist

Check before marking the PR as ready for review:

  • Will the PR name make sense to users?
  • Does the PR have a priority label?
  • Have you added or updated tests?
  • Is the documentation up to date?
For significant changes:
  • Is there a summary in the CHANGELOG?
  • Can these changes be split into multiple PRs?

If a checkbox isn't relevant to the PR, mark it as done.

Solution

Update zcash_script, restore windows support to CI and disable tests were needed.

Testing

Windows CI and @arya2 made some local checks in windows.

Review

Anyone can review.

Reviewer Checklist

Check before approving the PR:

  • Does the PR scope match the ticket?
  • Are there enough tests to make sure it works? Do the tests cover the PR motivation?
  • Are all the PR blockers dealt with?
    PR blockers can be dealt with in new tickets or PRs.

And check the PR Author checklist is complete.

Follow Up Work

@oxarbitrage oxarbitrage added the do-not-merge Tells Mergify not to merge this PR label Apr 11, 2024
@github-actions github-actions bot added the C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG label Apr 11, 2024
@arya2 arya2 removed the do-not-merge Tells Mergify not to merge this PR label Apr 17, 2024
Copy link
Contributor

@arya2 arya2 left a comment

Choose a reason for hiding this comment

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

We may be able to re-enable the rejection_restores_internal_state_genesis() test.

The logs suggest that the rpc_server_spawn_single_thread test is redundant on Windows, so I think it's fine to skip it, or to skip the multi-thread version of the test, either should work.

zebra-state/src/service/non_finalized_state/tests/prop.rs Outdated Show resolved Hide resolved
zebra-rpc/src/server/tests/vectors.rs Show resolved Hide resolved
@oxarbitrage oxarbitrage changed the title test(fix): Test if zcash script v0.1.15 can work. bump(zcash_script): Bump zcash script v0.1.15 and restore Windows support Apr 22, 2024
@oxarbitrage oxarbitrage marked this pull request as ready for review April 22, 2024 13:00
@oxarbitrage oxarbitrage requested review from a team as code owners April 22, 2024 13:00
@oxarbitrage oxarbitrage requested review from arya2 and upbqdn and removed request for a team April 22, 2024 13:00
@oxarbitrage oxarbitrage self-assigned this Apr 22, 2024
@oxarbitrage oxarbitrage added the A-dependencies Area: Dependency file updates label Apr 22, 2024
@oxarbitrage oxarbitrage added A-compatibility Area: Compatibility with other nodes or wallets, or standard rules and removed C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG labels Apr 22, 2024
Copy link
Contributor

@arya2 arya2 left a comment

Choose a reason for hiding this comment

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

Looks good

mergify bot added a commit that referenced this pull request Apr 22, 2024
@mergify mergify bot merged commit 8cf0b7a into main Apr 22, 2024
180 checks passed
@mergify mergify bot deleted the test_zcash_script_v0.1.15 branch April 22, 2024 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compatibility Area: Compatibility with other nodes or wallets, or standard rules A-dependencies Area: Dependency file updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants