-
Notifications
You must be signed in to change notification settings - Fork 102
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
feat: add fireblocks to aa-signers #301
Conversation
Current dependencies on/for this PR:
This stack of pull requests is managed by Graphite. |
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.
is this necessary? can't we just hardcode this value?
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.
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.
ah gotcha
|
||
You can either pass in a constructed `FireblocksWeb3Provider` object, or directly pass into the `FireblocksSigner` the `FireblocksProviderConfig` used to construct a `FireblocksWeb3Provider` object. These parameters are listed on the [Fireblocks repo](https://github.com/fireblocks/fireblocks-web3-provider/blob/main/src/types.ts#L48) as well. | ||
|
||
`FireblocksProviderConfig` takes in the following parameters: |
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.
oh oof... this is gonna be a pain to have to do and maintain for each signer provider in the future. I can see the devex benefit of having this here, but once it goes out of sync it causes more harm than good. can we link out to their types or docs instead?
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.
yeah where possible. some SDKs don't expose this, so I have to copy-paste onto the types.ts file for the Signer (so we'll still be out of sink).
but yeah, will just link to code where possible. (saves me time too 🙏 )
556b416
to
2a567a4
Compare
8ee5ca3
to
dee199a
Compare
dee199a
to
ccdca68
Compare
ccdca68
to
e55a747
Compare
might need to rebase again and then I can approve |
f369f33
to
6f369aa
Compare
Pull Request Checklist
yarn test
)site
folder, and guidelines for updating/adding docs can be found in the contribution guide)feat!: breaking change
)yarn lint:check
) and fix any issues? (yarn lint:write
)PR-Codex overview
Focus of this PR:
This PR focuses on adding the FireblocksSigner to the project and providing methods for authentication, getting addresses, signing messages, and signing typed data.
Detailed summary:
mock-private-key.txt
in thepackages/signers/src/fireblocks/__tests__/
directory.export { FireblocksSigner }
andexport type *
topackages/signers/src/fireblocks/index.ts
.import type { Address }
and interfacesFireblocksAuthenticationParams
andFireblocksUserInfo
topackages/signers/src/fireblocks/types.ts
.FireblocksSigner
andtype FireblocksAuthenticationParams
topackages/signers/src/index.ts
."@fireblocks/fireblocks-web3-provider": "^1.2.6"
topackages/signers/package.json
.site/.vitepress/config.ts
file.getAddress.md
,signMessage.md
,getAuthDetails.md
,authenticate.md
, andsignTypedData.md
files tosite/packages/aa-signers/fireblocks/
.fireblocks.ts
in thesite/snippets/
directory.import { FireblocksSigner }
andimport { ChainId }
tosite/snippets/fireblocks.ts
.createFireblocksSigner
function tosite/snippets/fireblocks.ts
.FireblocksSigner
class topackages/signers/src/fireblocks/signer.ts
.