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

signer module removed, since it's deprecated #366

Open
wants to merge 13 commits into
base: dev
Choose a base branch
from
Open

Conversation

pablodeymo
Copy link
Contributor

@pablodeymo pablodeymo commented Oct 1, 2024

Fixes # .

What Changed?

Signer module was revamped in #403. Users should move to signerv2, in case they weren't doing so. Instructions on how to move are in the module's readme.

Reviewer Checklist

  • Code is well-documented
  • Code adheres to Go naming conventions
  • Code deprecates any old functionality before removing it

@pablodeymo pablodeymo requested a review from afkbyte October 2, 2024 15:34
@pablodeymo pablodeymo added the v0.2 new dev version label Oct 14, 2024
@lferrigno
Copy link

There's a READMe.md file inside the signerv2 folder.

We should add a small description there on how it works and add a "How to upgrade from deprecated signer" section as well

@lferrigno
Copy link

There's a READMe.md file inside the signerv2 folder.

We should add a small description there on how it works and add a "How to upgrade from deprecated signer" section as well

Added

afkbyte
afkbyte previously approved these changes Jan 15, 2025
@afkbyte afkbyte dismissed their stale review January 15, 2025 21:11

accidental approval

Signers instantiated from this module is required to create some SDK transaction managers (see [`NewPrivateKeyWallet`](../chainio/clients/wallet/privatekey_wallet.go) and [`NewSimpleTxManager`](../chainio/txmgr/simple.go)/[`NewGeometricTxnManager`](../chainio/txmgr/geometric/geometric.go)).

## Features
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be beneficial to include examples for each of the different signing mechanisms, would provide better context for devs wanting to use our signer!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! We can tackle that in another PR. I created #440 to track it.

### Using SignerFromConfig

SignerV2 introduces `SignerFromConfig`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be nice to show and break down what the config contains here in the docs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the suggestion to #440

signerv2/README.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v0.2 new dev version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants