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

add pub functionality for zaino #8964

Merged
merged 4 commits into from
Nov 5, 2024

Conversation

idky137
Copy link
Contributor

@idky137 idky137 commented Oct 23, 2024

Motivation

For Zaino to use Zebra as its backed it requires access to primitives held in Zebra-Chain and Zebra-RPC.

Solution

-Several structs in zebra-rpc were made pub and getter / setter functionality was added.
-From impl was added to ['ConsensusBranchId'] in zebra-chain.

Tests

The most complex functionality added is simple getter / setter impls, I believe that adding new tests for this would be inefficient / redundant.

Follow-up Work

Due to patched libraries in Zebra:main causing issues in Zaino these changes were made atop v1.9.0, once the patches are removed we will update the zebra version to 2.0.0, there is a chance that additional changes will be required for changes in zebra v2.0.0.

PR Author's Checklist

  • The PR name will make sense to users.
  • The PR provides a CHANGELOG summary.
  • The solution is tested.
  • The documentation is up to date.
  • The PR has a priority label.

PR Reviewer's Checklist

  • The PR Author's checklist is complete.
  • The PR resolves the issue.

@idky137 idky137 marked this pull request as ready for review October 23, 2024 19:41
@idky137 idky137 requested a review from a team as a code owner October 23, 2024 19:41
@idky137 idky137 requested review from oxarbitrage and removed request for a team October 23, 2024 19:41
@idky137
Copy link
Contributor Author

idky137 commented Oct 24, 2024

I'm not sure whether these changes require a CHANGELOG entry?

oxarbitrage
oxarbitrage previously approved these changes Oct 24, 2024
Copy link
Contributor

@oxarbitrage oxarbitrage 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 to me, i left a minor suggestion for a doc change.

@gustavovalverde is working on a CI issue we have with external pull requests so we can merge this one.

Thanks!

zebra-rpc/src/methods.rs Outdated Show resolved Hide resolved
@arya2 arya2 added the do-not-merge Tells Mergify not to merge this PR label Oct 26, 2024
@idky137
Copy link
Contributor Author

idky137 commented Oct 26, 2024

Please let me know if there is anything I can do to help remove the 'Do-Not-Merge' Tag..

@mpguerra
Copy link
Contributor

Please let me know if there is anything I can do to help remove the 'Do-Not-Merge' Tag..

It was just while we were prepping the release. We should be good to merge now that's out :)

@mpguerra mpguerra removed the do-not-merge Tells Mergify not to merge this PR label Oct 28, 2024
@oxarbitrage
Copy link
Contributor

Hi @idky137 can you update your branch with the last main ? We just merged some fixes to the external contributor workflow and we want to check if it is working.

@mergify mergify bot merged commit c26c3f2 into ZcashFoundation:main Nov 5, 2024
86 of 87 checks passed
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