Skip to content

refactor: extract shared identity/signer setup into helpers#38

Open
thephez wants to merge 1 commit intomasterfrom
v3-refactor-boilerplate
Open

refactor: extract shared identity/signer setup into helpers#38
thephez wants to merge 1 commit intomasterfrom
v3-refactor-boilerplate

Conversation

@thephez
Copy link
Collaborator

@thephez thephez commented Feb 3, 2026

Add prepareTransition, getIdentityKey, and createSigner helper functions to eliminate duplicated boilerplate across 18 state transition operations.

Reduces app.js by ~330 lines while maintaining identical behavior.

All tests pass locally.

Co-Authored-By: Claude Opus 4.5 noreply@anthropic.com

Summary by CodeRabbit

  • Refactor
    • Unified signing and transaction preparation across contract, document, token and transfer operations for more consistent behavior.
    • Standardized cryptographic handling and centralized error messages for missing keys or identities.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 3, 2026

📝 Walkthrough

Walkthrough

Centralized identity key selection and signer creation into getIdentityKey, createSigner, and prepareTransition; replaced per-operation signer setup across data contract, document, and token operations; introduced DEFAULT_SECURITY_LEVELS and CONTRACT_SECURITY_LEVELS; expanded imports to include DataContract, Document, IdentitySigner, and Identifier. Net change: +85/-417 (−332 lines).

Changes

Cohort / File(s) Summary
Refactored Signer Preparation & State Transition
public/app.js
Added DEFAULT_SECURITY_LEVELS and CONTRACT_SECURITY_LEVELS. Introduced getIdentityKey, createSigner, and prepareTransition to centralize identity/key selection, signer instantiation, and error handling. Replaced ad-hoc signer setup across dataContractCreate/update, documentCreate/replace/delete/transfer/purchase/setPrice, and token operations with prepareTransition usage. Unified entropy/documentId and BigInt handling. Updated import to include DataContract, Document, IdentitySigner, Identifier.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: extracting shared identity/signer setup logic into reusable helper functions, which is the core purpose of this refactoring.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch v3-refactor-boilerplate

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@thephez thephez changed the base branch from master to chore/update-to-2.2 February 3, 2026 21:30
Base automatically changed from chore/update-to-2.2 to master February 5, 2026 02:07
@thephez thephez marked this pull request as draft February 5, 2026 14:37
Add prepareTransition, getIdentityKey, and createSigner helper functions
to eliminate duplicated boilerplate across 18 state transition operations.
Reduces app.js by ~330 lines while maintaining identical behavior.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@public/app.js`:
- Around line 1511-1540: The security-level arrays DEFAULT_SECURITY_LEVELS and
CONTRACT_SECURITY_LEVELS are using string values while getIdentityKey compares
against numeric k.securityLevel; update those constant arrays to use the numeric
enum values (e.g., 1,2,3 for Critical/High/Medium and 1,2 for Critical/High) so
the includes() check in getIdentityKey will succeed; verify no other code
expects string labels and adjust any references to DEFAULT_SECURITY_LEVELS or
CONTRACT_SECURITY_LEVELS accordingly.
🧹 Nitpick comments (1)
public/app.js (1)

2236-2238: Remove unused identity from destructuring.

The identity variable is destructured but never used in this block—n.ownerId is used directly for all identity-related operations. Consider matching the pattern used in dataContractUpdate (line 2322).

Suggested fix
-      const { identity, identityKey, signer } = await prepareTransition(
+      const { identityKey, signer } = await prepareTransition(
         c, n.ownerId, n.privateKeyWif, n.keyId, CONTRACT_SECURITY_LEVELS
       );

@thephez thephez force-pushed the v3-refactor-boilerplate branch from b261636 to 72cab62 Compare February 5, 2026 14:48
@thephez thephez marked this pull request as ready for review February 5, 2026 14:55
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.

1 participant