Skip to content

feat(sdk-core): add EdDSA MPCv2 offline signing helper utils#8793

Open
vibhavgo wants to merge 1 commit into
masterfrom
WCI-386/eddsa-util-helper
Open

feat(sdk-core): add EdDSA MPCv2 offline signing helper utils#8793
vibhavgo wants to merge 1 commit into
masterfrom
WCI-386/eddsa-util-helper

Conversation

@vibhavgo
Copy link
Copy Markdown
Contributor

Adds shared private utility methods to EddsaMPCv2Utils that will be used across all three createOfflineRound1/2/3Share methods. These are prerequisite helpers that centralize transaction payload extraction, GPG key handling, and authenticated data validation to prevent code duplication.

  • Add domain-separator constant MPS_DSG_SIGNING_USER_GPG_KEY for adata prefixes
  • Add getSignableHexAndDerivationPath() to extract signableHex and derivationPath from txRequest
  • Add getBitgoAndUserGpgKeys() to decrypt user GPG keys with v1 (SJCL) and v2 (Argon2id) envelope support
  • Add validateAdata() to validate authenticated data matches cyphertext adata
  • Import isV2Envelope from baseTypes for envelope version detection
  • Add comprehensive test coverage for all three helper methods in createKeychains.ts

Ticket: WCI-386

@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 18, 2026

WCI-386

@vibhavgo vibhavgo changed the title feat(sdk-core): add EdDSA MPCv2 offline signing helper infrastructure feat(sdk-core): add EdDSA MPCv2 offline signing helper util May 18, 2026
@vibhavgo vibhavgo changed the title feat(sdk-core): add EdDSA MPCv2 offline signing helper util feat(sdk-core): add EdDSA MPCv2 offline signing helper utils May 18, 2026
@vibhavgo vibhavgo force-pushed the WCI-386/eddsa-util-helper branch from 73179bb to f4ebbae Compare May 18, 2026 10:40
@vibhavgo vibhavgo marked this pull request as ready for review May 18, 2026 11:04
@vibhavgo vibhavgo requested review from a team as code owners May 18, 2026 11:04
@vibhavgo vibhavgo requested review from Marzooqa and bdesoky May 18, 2026 11:04
Comment on lines +537 to +540
private getSignableHexAndDerivationPath(txRequest: TxRequest): {
signableHex: string;
derivationPath: string;
} {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it possible to extract this? Lets have like a common utils for ecdsa and eddsa if applicable

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Extracted common

* @returns void
* @throws {Error} if the adata or cyphertext is invalid
*/
private validateAdata(adata: string, cyphertext: string, roundDomainSeparator: string): void {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This as well?

txRequest.transactions && txRequest.transactions.length === 1,
'createOfflineShare requires exactly one transaction in txRequest'
);
const unsignedTx = txRequest.transactions[0].unsignedTx;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing null-check on unsignedTx before accessing its properties. If transactions[0].unsignedTx is null/undefined, this throws a generic TypeError instead of the descriptive assert message. Add assert(unsignedTx, 'Missing unsignedTx in transaction') before the two property asserts.

decryptedGpgPrvKey = this.bitgo.decrypt({ input: encryptedUserGpgPrvKey, password: walletPassphrase });
}

this.validateAdata(adata, encryptedUserGpgPrvKey, EddsaMPCv2Utils.MPS_DSG_SIGNING_USER_GPG_KEY);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug + inconsistency with ECDSA: EcdsaMPCv2Utils.getBitgoAndUserGpgKeys wraps this call with if (adata) { ... }. Without that guard, v1 SJCL ciphertexts encrypted without adata will always throw "Adata does not match cyphertext adata" when a non-empty adata is passed in — which breaks the v1 test in the test file.

Please add:

if (adata) {
  this.validateAdata(adata, encryptedUserGpgPrvKey, EddsaMPCv2Utils.MPS_DSG_SIGNING_USER_GPG_KEY);
}

* @returns void
* @throws {Error} if the adata or cyphertext is invalid
*/
private validateAdata(adata: string, cyphertext: string, roundDomainSeparator: string): void {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is byte-for-byte identical to EcdsaMPCv2Utils.validateAdata. Both classes share the same baseTSSUtils<KeyShare> ancestor, so this should be lifted there to eliminate the duplication. This is the most straightforward extraction — same signature, same body, no ECDSA/EdDSA differences.

const encryptedUserGpgPrvKey = bitgo.encrypt({
input: userGpgKeyPair.privateKey,
password: passphrase,
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This test will fail as written. The ciphertext is encrypted without adata, but adata = 'test-adata' is passed to getBitgoAndUserGpgKeys on line 398. Since validateAdata is called unconditionally (no if (adata) guard), it will find cypherJson.adata is undefined, fail both comparisons, and throw "Adata does not match cyphertext adata".

Either:

  1. Add the if (adata) guard in the implementation (preferred, consistent with ECDSA), or
  2. Pass an empty string for adata in this test to reflect the v1 no-adata case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will add adata guard in impl

@vibhavgo vibhavgo force-pushed the WCI-386/eddsa-util-helper branch from f4ebbae to 2188290 Compare May 19, 2026 10:10
@vibhavgo vibhavgo requested a review from Marzooqa May 19, 2026 10:10
const encryptedUserGpgPrvKey = bitgo.encrypt({
input: userGpgKeyPair.privateKey,
password: passphrase,
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This test is still broken after the update. The ciphertext is encrypted without adata, but adata = 'test-adata' (truthy) is passed to getBitgoAndUserGpgKeys on line 398. The new if (adata) guard only skips validation when adata is falsy — 'test-adata' is not, so validateAdata still runs, finds cypherJson.adata is undefined/empty, fails both comparisons, and throws.

Fix: either encrypt with matching adata in this test, or pass '' (empty string) to getBitgoAndUserGpgKeys to reflect the no-adata v1 case:

const result = await (tssUtils as any).getBitgoAndUserGpgKeys(
  bitgoGpgKeyPair.publicKey,
  encryptedUserGpgPrvKey,
  passphrase,
  ''  // no adata for v1 SJCL
);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes
Addressed with two separate checks

Ticket: WCI-386

Adds EdDSA MPCv2 offline signing helper infrastructure and centralizes
common MPCv2 helper logic in BaseTssUtils for reuse across ECDSA and EdDSA.
The shared helpers cover transaction payload extraction and authenticated
data validation while keeping scheme-specific signing behavior local.

- Add MPS_DSG_SIGNING_USER_GPG_KEY domain-separator constant for adata prefixes
- Add getBitgoAndUserGpgKeys() to decrypt user GPG keys with v1 (SJCL)
  and v2 (Argon2id) envelope support
- Move getSignableHexAndDerivationPath() into BaseTssUtils for shared
  ECDSA and EdDSA MPCv2 transaction extraction
- Move validateAdata() into BaseTssUtils to eliminate duplicated
  authenticated data validation
- Reuse shared transaction extraction from ECDSA before scheme-specific hashing
- Import isV2Envelope from baseTypes for envelope version detection
- Add comprehensive test coverage for helper behavior
@vibhavgo vibhavgo force-pushed the WCI-386/eddsa-util-helper branch from 2188290 to 6340d35 Compare May 19, 2026 15:48
@vibhavgo vibhavgo requested a review from Marzooqa May 19, 2026 15:49
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