-
Notifications
You must be signed in to change notification settings - Fork 180
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
fix deterministic.rs 'seal changed -was: 0xa7a4 now 0x544d' #438
Conversation
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.
After a quick talk with @aspect I can confirm that changing the seal hash here is okay, as long as the fields did not change (which is the case).
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.
This allows compiling the entire repo (specifically the wallet part) with the latest rust version (1.77.0 as of this writing) but breaks older version.
kaspad and the rest of the components that do not depend on wallet are semantically the same and are not affected by this PR.
LGTM.
But what is this seal and its purpose and why does it suddenly got broken? Can we really just patch it with a new value? |
This pr is included by 1.77 rustc version I suggest to @aspect to make it derive macro only depending on fields and types, he replied that this is general purpose macro and it can be applied to any code. In summary it will work fine until compiler change the logic again. Imho the proper solution is changing the hash and pinning rust toolchain in the repo. And maybe adding msrv |
I see, thanks for clarification |
Thank you for your contribution. As this was integrated into a more fundamental fix in #442 this current PR has been superseded. |
Fixed in #442 I couldn't merge this PR due to other ongoing issues with lints (caused by the new rustc version as well), so the change was applied in the above-mentioned PR that was just merged. |
fix issue: #437
reason:
seal changed -was: 0xa7a4 now 0x544d