Skip to content

Fix DIF holder presentation signing for did:key - #4196

Merged
swcurran merged 9 commits into
openwallet-foundation:mainfrom
jacksonriding:fix/4195-dif-holder-signing
Sep 8, 2026
Merged

Fix DIF holder presentation signing for did:key#4196
swcurran merged 9 commits into
openwallet-foundation:mainfrom
jacksonriding:fix/4195-dif-holder-signing

Conversation

@jacksonriding

Copy link
Copy Markdown
Contributor

Fixes #4195.

Preserves explicit issuer_id when the original presentation definition is reused; honors explicit pres_signing_did before holder auto-detection; skips non-local subject DIDs while discovering a local did:key; and fails explicitly when holder signing cannot determine a local DID instead of returning an unsigned VP.

Validation: 74 affected DIF tests passed, 46 optional BBS tests skipped; Ruff passed; git diff --check passed.

Fixes openwallet-foundation#4195 by preserving explicit issuer_id, honoring explicit holder signing DIDs, improving automatic did:key discovery, and failing clearly when holder signing cannot determine a local DID.

Signed-off-by: jacksonriding <jackson.riding@gmail.com>
@jacksonriding
jacksonriding force-pushed the fix/4195-dif-holder-signing branch from 580c4c9 to a5ac02b Compare August 29, 2026 01:41
@jacksonriding

Copy link
Copy Markdown
Contributor Author

Forgot to sign :P

@swcurran

swcurran commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

The two failing tests look real -- could you @jacksonriding take a look at them? The ruff one should be trivial, but not sure about the PR Tests -- certainly seems to be in the area that you are updating.

@jacksonriding -- I'm assuming that once this PR is merged, it should be cherry-picked and applied to the 1.6.lts and 1.3.lts branches as well? Are you able to do that or should I plan to do it?

@jacksonriding

Copy link
Copy Markdown
Contributor Author

Thanks @swcurran. I tracked down and fixed both failures. Ruff was a formatting issue in the new test. The PR test
failures were caused by an incorrectly configured requirement mock and an existing suite-mismatch test that still expected an unsigned VP instead of the new explicit error behavior.

DIF suite now passes all tests with BBS. Ruff also passes.

Will look into backporting 1.6 and 1.3 lts after merge

@sonarqubecloud

sonarqubecloud Bot commented Sep 8, 2026

Copy link
Copy Markdown

@swcurran
swcurran merged commit c148063 into openwallet-foundation:main Sep 8, 2026
12 checks passed
swcurran pushed a commit that referenced this pull request Sep 9, 2026
Backport of #4196, merge commit c148063. Includes the complete production fix and regression tests.

Signed-off-by: Jackson Riding <jackson.riding@gmail.com>
swcurran pushed a commit that referenced this pull request Sep 9, 2026
Backport of #4196, merge commit c148063. Includes the complete production fix and regression tests.

Signed-off-by: Jackson Riding <jackson.riding@gmail.com>
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.

[BUG] DIF PE (ld_proof) holder always signs with auto-detected DID; explicit issuer_id in send-presentation is ignored

2 participants