Skip to content

chore: upgrade elliptic, k256 and yubi crates to latest#40

Merged
ameten merged 2 commits into
hyperlane-xyz:hyperlanefrom
Tessellated-io:tessellated/clean-crypto-upgrade
May 2, 2025
Merged

chore: upgrade elliptic, k256 and yubi crates to latest#40
ameten merged 2 commits into
hyperlane-xyz:hyperlanefrom
Tessellated-io:tessellated/clean-crypto-upgrade

Conversation

@keefertaylor

Copy link
Copy Markdown

Motivation

hyperlane-xyz/hyperlane-monorepo#5625 implements signing with a YubiHSM2 device in Hyperlane.

Yubihsm signing requires using the yubihsm feature from the yubi crate. Both yubi and various other parts of ethers-signers, ethers-core and hyperlane-* libraries require signing code to implement DigestSigner. This interface is closely coupled (AFAICT) with versions of elliptic and k256. In otherwords, both seem to need to be bumped at once.

A previous reviewer comment correctly pointed out that for implementing yubihsm2 signing in hyperlane, we should use a global library version of k256 and elliptic across the codebase.

As things stand today, we can use the existing versions in hyperlane and pull in the yubi crate at v0.41.x. Unfortunately, there is a bug in 0.41.x < yubi > 0.42.1 which causes a signing request to hang indefinitely (effectively deadlocking the process) in the case of a network failure.

Bumping yubi to v0.42.1 pulls in newer versions of elliptic and k256 which have replaced the DigestSigner interface with a similar but different PreHashSigner interface. I cannot figure out a way to get hyperlane to build with yubi@0.42.1 without having a consistent version of k256 and elliptic across ethers-rs and hyperlane-*. Therefore I think that we need to bump these packages as a prerequisite to hyperlane-xyz/hyperlane-monorepo#5625.

If you agree, I can make a corresponding PR to hyperlane-monorepo that shows we can bump these deps there without any code changes.

I ran clippy and there are lots of warnings, but I don't think they are related to changes in the PR (I see them on the main hyperlane branch as well). Likewise, all tests pass on this branch that are passing on the hyperlane branch.

Solution

This PR is a copy of the upstream PR which upgraded the same deps. See (upstream pr)[https://github.com/gakonst/pull/2260].

PR Checklist

  • [n/a] Added Tests
  • [n/a] Added Documentation
  • [n/a] Updated the changelog

@ameten

ameten commented Apr 29, 2025

Copy link
Copy Markdown

@keefertaylor , thank you for the PR. Much appreciated.

It is signing related, so, if you could raise a PR into this and monorepo with only dependency upgrades, it will be great. It is important to review the upgrade separately.

Comment thread ethers-core/src/utils/anvil.rs
Comment thread ethers-core/src/utils/ganache.rs
@keefertaylor

Copy link
Copy Markdown
Author

Converted over moonbeam.rs to using from_slice as well.

@keefertaylor keefertaylor force-pushed the tessellated/clean-crypto-upgrade branch from d4521f9 to 1da2c5f Compare May 2, 2025 19:09
@keefertaylor

Copy link
Copy Markdown
Author

Chatted offline and I reverted the latest push to stick to the upstream change.

@ameten ameten merged commit 602b5c7 into hyperlane-xyz:hyperlane May 2, 2025
github-merge-queue Bot pushed a commit to hyperlane-xyz/hyperlane-monorepo that referenced this pull request May 6, 2025
### Description

This is a bump of the `ethers-rs` deps to the latest tagged version. 

See:
- hyperlane-xyz/ethers-rs#40 for a description
of these changes
- #5625 for the
PR that will utilize the updated dependencies

### Drive-by changes

n/a

### Related issues

n/a

### Backward compatibility

Yes

### Testing

Manual testing via using #5625
ameten added a commit that referenced this pull request May 7, 2025
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.

2 participants