-
Notifications
You must be signed in to change notification settings - Fork 240
Assign Ed25519 precompile to native loader #417
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
base: main
Are you sure you want to change the base?
Assign Ed25519 precompile to native loader #417
Conversation
|
Hello deanmlittle! Welcome to the SIMD process. By opening this PR you are affirming that your SIMD has been thoroughly discussed and vetted in the SIMD discussion section. The SIMD PR section should only be used to submit a final technical specification for review. If your design / idea still needs discussion, please close this PR and create a new discussion here. This PR requires the following approvals before it can be merged:
Once all requirements are met, you can merge this PR by commenting |
|
i don't think this is necessary. it's only testnet. i'd far prefer to just rewrite the account on the next testnet restart than carry a testnet-only feature gate around. the inconsistencies in cluster feature sets have already been enough trouble historically |
topointon-jump
left a comment
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.
I think this is a good approach. For consistency's sake we should activate this feature on all networks, even it it will have no effect on devnet/mainnet.
Up to you ser. Just doing what I can to try make things less broken. Seems everyone I've asked is cool with it now. If this makes it into 3.1.0, I think it's worth it. If it doesn't, I think your approach is better. Having to wait for a whole release cycle to fix testnet is an unacceptable delay imo. |
|
As written, the SIMD feature gate can be activated on all clusters. It's also quite simple so I would be in favor of backporting the implementation to v3.1. @t-nelson thoughts? |
buffalojoec
left a comment
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.
Lgtm, I think the all-cluster activation method makes sense, too.
| and setting the data to `ed25519_program` to match all other clusters. All | ||
| other fields remain unchanged. | ||
|
|
||
| ```rust |
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.
I don't think the code snippet is valuable here. The spec should be agnostic anyway.
Co-authored-by: Joe C <[email protected]>
|
Thanks, topointon-jump! |
that's all good and well, but why? why waste all this time on testnet? we can just ram it in. who cares? |
I'd rather not use "hacks" to fix any cluster, including testnet. Using ledger tool to patch a cluster snapshot is pretty heavy handed for something trivial to fix via feature gate. And the fix via feature gate is simple enough to not waste much time. |
|
i would rather not no-op hard fork mainnet beta under any circumstances |
I don't really mind doing that. But your snapshot change looks fine to me too. It's not too different to how we deactivate features on testnet. Let's do that instead of a feature gate. |
|
yeah it's the same class of activity as "deactivate feature gate" or "forced destake" imo. if this was any other cluster i'd be more hestitant, but testnet is here for validator client dev convenience and a feature gate is not convenient |
No description provided.