-
Notifications
You must be signed in to change notification settings - Fork 2
feat: rework signer and enable indirect signing #47
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
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #47 +/- ##
==========================================
+ Coverage 99.01% 99.10% +0.09%
==========================================
Files 144 147 +3
Lines 7175 7285 +110
Branches 732 733 +1
==========================================
+ Hits 7104 7220 +116
+ Misses 71 65 -6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
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.
Pull request overview
This PR refactors the signer infrastructure to enable indirect signing capabilities, where a signer can sign on behalf of multiple targets (public keys or contract IDs) beyond just its own public key. The changes include renaming TransactionSigner to Signer, adding target management to LocalSigner, reorganizing type guards from the verifiers directory to type-guards, and enhancing the isDefined utility to check for both undefined and null.
- Introduced
signsFor()method to theSignerinterface for flexible target-based signing - Added target management methods (
addTarget,removeTarget,getTargets) toLocalSigner - Enhanced
isDefinedtype guard to check for bothnullandundefined(previously onlyundefined) - Created new error handling classes for signer operations (SECRET_NOT_ACCESSIBLE, SIGNER_DESTROYED, CANNOT_REMOVE_MASTER_TARGET)
Reviewed changes
Copilot reviewed 42 out of 46 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| core/deno.json | Version bump to 0.13.0 |
| plugins/fee-bump/deno.json | Version bump to 0.5.0, updated core dependency to ^0.13.0 |
| event-streamer/deno.json | Version bump to 0.4.0, updated core dependency to ^0.13.0 |
| core/signer/types.ts | Renamed TransactionSigner to Signer, added signsFor() method and ContractId support |
| core/signer/local/types.ts | Updated to use Signer type |
| core/signer/local/index.ts | Added target management, secretKey() method with hiding option, updated error handling |
| core/signer/local/index.unit.test.ts | Comprehensive tests for new target management and secretKey features |
| core/signer/local/error.ts | New error classes for local signer operations |
| core/signer/error.ts | Base error class for signer domain |
| core/common/type-guards/is-defined.ts | Enhanced to check both null and undefined |
| core/common/type-guards/is-signer.ts | New type guard for Signer interface including signsFor |
| core/common/type-guards/has-function.ts | Helper for checking function existence on objects |
| core/common/type-guards/*.ts | Moved from verifiers directory with updated implementations |
| core/common/verifiers/* | Removed in favor of type-guards |
| core/common/index.ts | Updated exports to use type-guards |
| core/processes/sign-envelope/* | Updated to use Signer type and signsFor() method |
| core/processes/sign-auth-entries/* | Updated to use Signer type and signsFor() method |
| core/processes/wrap-fee-bump/* | Updated imports to use type-guards |
| core/processes/envelope-signing-requirements/* | Updated imports to use type-guards |
| core/processes/assemble-transaction/index.ts | Updated imports to use type-guards |
| core/transformers/pipeline-connectors/index.unit.test.ts | Updated imports to use type-guards |
| core/network/index.ts | Updated imports to use type-guards |
| core/event/parsing/ledger-close-meta.ts | Updated imports to use type-guards |
| core/event/event.ts | Updated imports to use type-guards |
| core/error/types.ts | Added 'signer' error domain |
| core/error/index.unit.test.ts | Updated test to use 'account' domain instead of 'accounts' |
| core/account/native/* | Updated to use Signer type |
| core/asset/sac/* | Updated imports and types |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This pull request refactors the codebase to replace the
TransactionSignertype with a more genericSignertype across all modules, and consolidates various "verifier" utility functions under a new "type-guards" directory for improved clarity and maintainability. It also introduces new utility type guards and removes redundant code. These changes enhance type safety, code organization, and future extensibility.Type system and API refactor:
TransactionSignerwith the more generalSignertype in account, asset, and transaction config modules, including method signatures and type definitions (core/account/native/index.ts,core/account/native/types.ts,core/asset/sac/index.integration.test.ts,core/asset/sac/index.ts,core/common/types/transaction-config/types.ts) [1] [2] [3] [4] [5] [6] [7] [8] [9] [10].Type guard consolidation and improvements:
Moved all "verifier" functions to a new "type-guards" directory, updating imports and exports throughout the codebase (
core/common/index.ts,core/common/type-guards/index.ts,core/common/type-guards/index.unit.test.ts,core/common/type-guards/is-smart-contract-transaction.ts,core/common/verifiers/index.ts,core/common/verifiers/is-defined.ts,core/common/verifiers/is-transaction-signer.ts) [1] [2] [3] [4] [5] [6] [7].Added new type guard utilities:
isDefined,hasFunction, andisSignerfor more robust runtime type checking (core/common/type-guards/is-defined.ts,core/common/type-guards/has-function.ts,core/common/type-guards/is-signer.ts) [1] [2] [3].Version update:
0.13.0incore/deno.jsonto reflect these breaking and structural changes.