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 Fireblocks Signer #706

Merged
merged 8 commits into from
Oct 4, 2023
Merged

Update Fireblocks Signer #706

merged 8 commits into from
Oct 4, 2023

Conversation

amiecorso
Copy link
Contributor

@amiecorso amiecorso commented Sep 28, 2023

NO-3258

Updates the contracts repo hardhat infrastructure away from our old custom Fireblocks signer to instead use the recommended hardhat integration (which didn't exist when we were initially building this so Scott put together something custom). We also extend the base implementation of the fireblocks-hardhat integration to implement complete ethers Signer functionality. The custom integration was no longer working when we revisited it this month to perform some script-based listing and while it's not clear whether it's deprecated beyond salvation (for example, if the Fireblocks rest API is no longer live, then it is) we figure the easier and likely inevitable fix is to migrate to this finally mature third-party library created by Fireblocks, especially given the complexity of the custom implementation.

@openzeppelin-code
Copy link

openzeppelin-code bot commented Sep 28, 2023

Update Fireblocks Signer

Generated at commit: 965c202b518259f62820cf2dc557fffad3583116

🚨 Vulnerabilities Summary

Process Issues Results
Contract Inspector note
low
Total
21
9
30
Dependency Checker Total 0

For more details view the full report in OpenZeppelin Code

@swarmia
Copy link

swarmia bot commented Sep 28, 2023

✅  Linked to Task NO-3258 · Update Fireblocks signer in contracts repo

@codecov
Copy link

codecov bot commented Sep 28, 2023

Codecov Report

Merging #706 (965c202) into master (d9a135d) will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master    #706   +/-   ##
======================================
  Coverage    54.7%   54.7%           
======================================
  Files          14      14           
  Lines         979     979           
  Branches      152     152           
======================================
  Hits          536     536           
  Misses        386     386           
  Partials       57      57           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

package.json Outdated Show resolved Hide resolved
plugins/index.ts Outdated Show resolved Hide resolved
plugins/index.ts Outdated Show resolved Hide resolved
plugins/index.ts Outdated Show resolved Hide resolved
plugins/index.ts Outdated Show resolved Hide resolved
tasks/utils/contract-functions.ts Show resolved Hide resolved
tasks/vesting.ts Outdated Show resolved Hide resolved
tasks/vesting.ts Outdated Show resolved Hide resolved
types/global.d.ts Outdated Show resolved Hide resolved
types/global.d.ts Show resolved Hide resolved
@amiecorso amiecorso requested a review from jaycenhorton October 3, 2023 21:55
jaycenhorton
jaycenhorton previously approved these changes Oct 4, 2023
plugins/fireblocks/index.ts Outdated Show resolved Hide resolved
plugins/fireblocks/index.ts Outdated Show resolved Hide resolved
plugins/fireblocks/type-extensions.ts Show resolved Hide resolved
@amiecorso amiecorso requested a review from jaycenhorton October 4, 2023 14:42
@amiecorso amiecorso merged commit fcc94ad into master Oct 4, 2023
7 of 8 checks passed
@amiecorso amiecorso deleted the amie-update-fireblocks-signer branch October 4, 2023 14:46
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