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

Update bitflags to v2 #5669

Closed
stringhandler opened this issue Aug 27, 2023 · 0 comments · Fixed by #5831
Closed

Update bitflags to v2 #5669

stringhandler opened this issue Aug 27, 2023 · 0 comments · Fixed by #5831
Assignees
Labels
release-blocker Something that needs to be fixed before a release can be made

Comments

@stringhandler
Copy link
Collaborator

bitflags has released a v2. We are currently using v1.

@SWvheerden SWvheerden added the release-blocker Something that needs to be fixed before a release can be made label Sep 19, 2023
@brianp brianp self-assigned this Oct 4, 2023
SWvheerden added a commit that referenced this issue Oct 11, 2023
Description
---
Upgrade the bitflags crate to 2.4.0. This made a few notable changes:
- serde serialization changed from a map `{"bits": 0}` to just
outputting the compact bit `0`. It is possible to maintain the legacy
format but we don't need to keep that.
- bitflags maintain an internal representation which Borsh can't
serialize. So in the instance of KernerlFeatures the interface was
migrated from the use of the standard bitflags macro to defining the
struct ourself, and implementing a bitflags trait. This allows borsh to
operate as normal on the unit struct wrapper of the u8 rep.
- The kernel feature string representation in the faucet file required
updating, but the value is still the same once deserialized so it
shouldn't require a change in the genesis block, or tx's.
- It will still be a breaking change as old nodes and new nodes would
have a different serialized representation over rpc, and grpc.
- the bitflags crate no longer derives on the structs it creates. I've
manually added only the needed derivations to build, and pass tests.

Motivation and Context
---
We wanted to upgrade before release.
Closes: #5669 

[See v2 release changes for
bitflags](https://github.com/bitflags/bitflags/releases/tag/2.0.0)

How Has This Been Tested?
---
CI

What process can a PR reviewer use to test or verify this change?
---
Review the changes

<!-- Checklist -->
<!-- 1. Is the title of your PR in the form that would make nice release
notes? The title, excluding the conventional commit
tag, will be included exactly as is in the CHANGELOG, so please think
about it carefully. -->


Breaking Changes
---

- [ ] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [x] Other - May not be able to communicate with older nodes.

BREAKING CHANGE: May not be able to communicate with older nodes. Will
require users to upgrade their nodes

---------

Co-authored-by: SW van Heerden <swvheerden@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-blocker Something that needs to be fixed before a release can be made
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants