fixed duplicate present-proof v2 webhook #3998

sonivijayk · 2025-12-23T22:52:58Z

Fix Summary

Used by_format presence to drop legacy pres_* fields from webhook
Added test for by_format logic based legacy fields exclusion from payload, used dummy payload

Fix Testing

Ran : python -m pytest acapy_agent/protocols/present_proof/v2_0/models/tests/test_record.py

Signed-off-by: Vijay Soni <[email protected]>

* Fix issues with anoncreds upgrade Signed-off-by: jamshale <[email protected]> * Add additional test coverage for revocation after upgrade Signed-off-by: jamshale <[email protected]> --------- Signed-off-by: jamshale <[email protected]>

…enwallet-foundation#3993) Bumps the pip group with 1 update in the / directory: [marshmallow](https://github.com/marshmallow-code/marshmallow). Updates `marshmallow` from 3.26.1 to 3.26.2 - [Changelog](https://github.com/marshmallow-code/marshmallow/blob/3.26.2/CHANGELOG.rst) - [Commits](marshmallow-code/marshmallow@3.26.1...3.26.2) --- updated-dependencies: - dependency-name: marshmallow dependency-version: 3.26.2 dependency-type: direct:production dependency-group: pip ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: jamshale <[email protected]>

…t-foundation#3997) * docs: remove indy wallet usage from README.md (issue 2319) Signed-off-by: Vijay Soni <[email protected]> * docs: removed indy sdk reference from Present the Proof section of AliceGetsAPhone.md (issue 2319) Signed-off-by: Vijay Soni <[email protected]> * removed did:sov from AliceWantsAJsonCredential.md Signed-off-by: Vijay Soni <[email protected]> * replaced did:sov example with did:peer in ReusingAConnection.md Signed-off-by: Vijay Soni <[email protected]> * Removed indy sdk image reference from container docs Signed-off-by: Vijay Soni <[email protected]> * Updated deploymentModel.md storage wording Signed-off-by: Vijay Soni <[email protected]> * Replaced did:sov kid with did:key in SD-JWT example Signed-off-by: Vijay Soni <[email protected]> * Added clarification on Askar storage support in SupportedRFCs.md Signed-off-by: Vijay Soni <[email protected]> * Removed did:sov info from JsonLdCredentials.md Signed-off-by: Vijay Soni <[email protected]> * Minor change to remove did:sov from Registering a DID method section Signed-off-by: Vijay Soni <[email protected]> --------- Signed-off-by: Vijay Soni <[email protected]> Co-authored-by: Vijay Soni <[email protected]> Co-authored-by: jamshale <[email protected]>

…ates (openwallet-foundation#4003) Bumps the all-actions group with 3 updates in the / directory: [github/codeql-action](https://github.com/github/codeql-action), [docker/setup-buildx-action](https://github.com/docker/setup-buildx-action) and [dawidd6/action-download-artifact](https://github.com/dawidd6/action-download-artifact). Updates `github/codeql-action` from 4.31.8 to 4.31.9 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@1b168cd...5d4e8d1) Updates `docker/setup-buildx-action` from 3.11.1 to 3.12.0 - [Release notes](https://github.com/docker/setup-buildx-action/releases) - [Commits](docker/setup-buildx-action@e468171...8d2750c) Updates `dawidd6/action-download-artifact` from 11 to 12 - [Release notes](https://github.com/dawidd6/action-download-artifact/releases) - [Commits](dawidd6/action-download-artifact@ac66b43...0bd50d5) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 4.31.9 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: all-actions - dependency-name: docker/setup-buildx-action dependency-version: 3.12.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: all-actions - dependency-name: dawidd6/action-download-artifact dependency-version: '12' dependency-type: direct:production update-type: version-update:semver-major dependency-group: all-actions ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: jamshale <[email protected]>

…on#4000) Signed-off-by: jamshale <[email protected]>

* feat: Only log failing scenarios Signed-off-by: jamshale <[email protected]> * fix: Remove silent mode Signed-off-by: jamshale <[email protected]> --------- Signed-off-by: jamshale <[email protected]>

Signed-off-by: Vijay Soni <[email protected]>

…webhook-dedup

jamshale · 2026-01-06T17:13:30Z

I think this is looking good. My only reservation is that it would be considered a breaking change and I'm hesitant to include it in the next release.

I'll review it closer and we can probably get it included in the next, next release.

Also the DCO will need to get fixed.

Copilot

Pull request overview

This PR fixes a webhook duplication issue (BUG #3802) where present-proof v2 webhooks included both legacy pres_* fields and the newer by_format structure, causing duplicate data in webhook payloads.

Key Changes:

Modified webhook emission logic to conditionally exclude legacy fields when by_format is present in debug mode
Extracted and centralized _presentation_request_payload helper function to handle both by_format and legacy payload formats
Added comprehensive test coverage for the webhook payload stripping behavior

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`acapy_agent/protocols/present_proof/v2_0/models/pres_exchange.py`	Added conditional logic to strip legacy `pres_*` fields from webhook payloads when `by_format` is present and debug webhooks are enabled
`acapy_agent/protocols/present_proof/v2_0/models/tests/test_record.py`	Added test case to verify legacy fields are excluded from webhook payloads when `by_format` exists
`scenarios/examples/util.py`	Added `_presentation_request_payload` helper function and new `indy_present_proof_v2` function; updated `anoncreds_presentation_summary` and `anoncreds_present_proof_v2` to use the helper
`scenarios/examples/simple_restart/example.py`	Updated import to use `indy_present_proof_v2` from util module
`scenarios/examples/multitenancy/example.py`	Updated import to use `indy_present_proof_v2` from util module
`scenarios/examples/presenting_revoked_credential/example.py`	Added local `_presentation_request_payload` function and updated `summary` function to use it; updated import for `indy_present_proof_v2`
`scenarios/examples/kanon_issuance_and_presentation/example.py`	Added local `_presentation_request_payload` function and updated `summary` function to use it
`scenarios/examples/did_indy_issuance_and_revocation/example.py`	Added local `_presentation_request_payload` function and updated `summary` function to use it; updated import for `indy_present_proof_v2`

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

acapy_agent/protocols/present_proof/v2_0/models/pres_exchange.py

Copilot · 2026-01-06T17:17:38Z

scenarios/examples/presenting_revoked_credential/example.py

+from examples.util import indy_present_proof_v2

 ALICE = getenv("ALICE", "http://alice:3001")
 BOB = getenv("BOB", "http://bob:3001")


+def _presentation_request_payload(presentation: V20PresExRecord):
+    if presentation.by_format and presentation.by_format.pres_request:
+        return presentation.by_format.pres_request
+    request = presentation.pres_request
+    if not request:
+        return None
+    if isinstance(request, dict):
+        return request
+    if hasattr(request, "model_dump"):
+        return request.model_dump(by_alias=True)
+    return request.dict(by_alias=True)
+
+


The function _presentation_request_payload is duplicated in this file. It already exists in scenarios/examples/util.py (lines 63-75) with identical implementation. Consider removing this duplicate and importing it from util.py instead to maintain DRY principles and reduce maintenance burden.

Suggested change

from examples.util import indy_present_proof_v2

ALICE = getenv("ALICE", "http://alice:3001")

BOB = getenv("BOB", "http://bob:3001")

def _presentation_request_payload(presentation: V20PresExRecord):

if presentation.by_format and presentation.by_format.pres_request:

return presentation.by_format.pres_request

request = presentation.pres_request

if not request:

return None

if isinstance(request, dict):

return request

if hasattr(request, "model_dump"):

return request.model_dump(by_alias=True)

return request.dict(by_alias=True)

from examples.util import indy_present_proof_v2, _presentation_request_payload

ALICE = getenv("ALICE", "http://alice:3001")

BOB = getenv("BOB", "http://bob:3001")

Copilot · 2026-01-06T17:17:39Z

scenarios/examples/kanon_issuance_and_presentation/example.py

+def _presentation_request_payload(presentation: V20PresExRecord):
+    if presentation.by_format and presentation.by_format.pres_request:
+        return presentation.by_format.pres_request
+    request = presentation.pres_request
+    if not request:
+        return None
+    if isinstance(request, dict):
+        return request
+    if hasattr(request, "model_dump"):
+        return request.model_dump(by_alias=True)
+    return request.dict(by_alias=True)


The function _presentation_request_payload is duplicated in this file. It already exists in scenarios/examples/util.py (lines 63-75) with identical implementation. Consider removing this duplicate and importing it from util.py instead to maintain DRY principles and reduce maintenance burden.

Copilot · 2026-01-06T17:17:39Z

scenarios/examples/did_indy_issuance_and_revocation/example.py

+from examples.util import indy_present_proof_v2

 ALICE = getenv("ALICE", "http://alice:3001")
 BOB = getenv("BOB", "http://bob:3001")


+def _presentation_request_payload(presentation: V20PresExRecord):
+    if presentation.by_format and presentation.by_format.pres_request:
+        return presentation.by_format.pres_request
+    request = presentation.pres_request
+    if not request:
+        return None
+    if isinstance(request, dict):
+        return request
+    if hasattr(request, "model_dump"):
+        return request.model_dump(by_alias=True)
+    return request.dict(by_alias=True)
+
+


The function _presentation_request_payload is duplicated in this file. It already exists in scenarios/examples/util.py (lines 63-75) with identical implementation. Consider removing this duplicate and importing it from util.py instead to maintain DRY principles and reduce maintenance burden.

Suggested change

from examples.util import indy_present_proof_v2

ALICE = getenv("ALICE", "http://alice:3001")

BOB = getenv("BOB", "http://bob:3001")

def _presentation_request_payload(presentation: V20PresExRecord):

if presentation.by_format and presentation.by_format.pres_request:

return presentation.by_format.pres_request

request = presentation.pres_request

if not request:

return None

if isinstance(request, dict):

return request

if hasattr(request, "model_dump"):

return request.model_dump(by_alias=True)

return request.dict(by_alias=True)

from examples.util import indy_present_proof_v2, _presentation_request_payload

ALICE = getenv("ALICE", "http://alice:3001")

BOB = getenv("BOB", "http://bob:3001")

Copilot · 2026-01-06T17:17:39Z

scenarios/examples/util.py

 # anoncreds utilities:
+def _presentation_request_payload(
+    presentation: V20PresExRecord,
+) -> Optional[Dict[str, Any]]:


The function _presentation_request_payload is missing a docstring. Add a docstring that explains its purpose, parameters, and return value. For example: "Extract presentation request payload from a presentation exchange record, preferring by_format if available, falling back to legacy pres_request."

Suggested change

) -> Optional[Dict[str, Any]]:

) -> Optional[Dict[str, Any]]:

"""Extract presentation request payload from a presentation exchange record.

This helper prefers the `by_format.pres_request` field when available and

falls back to the legacy `pres_request` field. The legacy value may be a

plain dict or a model instance; in the latter case it is converted to a

dict representation using alias field names.

Args:

presentation: The v2.0 presentation exchange record containing the

formatted and/or legacy presentation request data.

Returns:

A dictionary representation of the presentation request payload, or

None if no request is available on the record.

"""

…webhook-dedup

sonarqubecloud · 2026-01-06T17:32:17Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

ff137 · 2026-01-06T18:19:46Z

Re: review request, I'll be able to take a look tomorrow 👌

ff137

Just some minor suggestions before approval

ff137 · 2026-01-07T11:29:06Z

acapy_agent/protocols/present_proof/v2_0/models/pres_exchange.py

+            # BUG #3802: remove legacy fields when by_format is present
+        elif payload.get("by_format"):
+            for key in ("pres_proposal", "pres_request", "pres"):
+                payload.pop(key, None)


if not session.profile.settings.get("debug.webhooks"): ... # BUG #3802: remove legacy fields when by_format is present elif payload.get("by_format"): for key in ("pres_proposal", "pres_request", "pres"): payload.pop(key, None)

->

if not session.profile.settings.get("debug.webhooks"): ... elif by_format := payload.get("by_format"): # Issue #3802: remove legacy fields when by_format is present for key in ("pres_proposal", "pres_request", "pres"): if key in by_format: payload.pop(key, None)

^ Move/reword comment; assign by_format; and check key in by_format before removing.

It's probably good to explicitly check that they key being removed does in fact exist in by_format already.

ff137 · 2026-01-07T11:33:50Z

acapy_agent/protocols/present_proof/v2_0/models/tests/test_record.py

+            payload = session.emit_event.call_args.args[1]
+            assert "by_format" in payload
+            assert "pres_request" not in payload
+            assert "pres" not in payload
+            assert "pres_proposal" not in payload


I would just add an assertion that those 3 keys do exist in payload["by_format"]

for key in ("pres", "pres_proposal", "pres_request"): assert key not in payload and key in payload["by_format"]

ff137 · 2026-01-07T11:46:23Z

scenarios/examples/util.py

+    assert request_payload
+    if "anoncreds" in request_payload or "indy" in request_payload:
+        proof_request = request_payload.get("anoncreds") or request_payload.get("indy")
+    else:
+        proof_request = request_payload
+    assert proof_request


There's already assert request_payload, so the else block is a bit redundant. Presumably can just drop the else, and move the 2nd assert into the if "anoncreds" in ... block:

if "anoncreds" in request_payload or "indy" in request_payload: proof_request = request_payload.get("anoncreds") or request_payload.get("indy") assert proof_request

ff137 · 2026-01-07T11:48:30Z

scenarios/examples/util.py

+    if "anoncreds" in request_payload or "indy" in request_payload:
+        proof_request = request_payload.get("indy") or request_payload.get("anoncreds")
+    else:
+        proof_request = request_payload
+    assert proof_request


Similar here -- proof_request isn't used later, so removing else block would just be a lil bit clearer

ff137 · 2026-01-07T11:49:38Z

scenarios/examples/util.py

+async def indy_present_proof_v2(
+    holder: Controller,
+    verifier: Controller,
+    holder_connection_id: str,
+    verifier_connection_id: str,
+    *,
+    name: Optional[str] = None,
+    version: Optional[str] = None,
+    comment: Optional[str] = None,
+    requested_attributes: Optional[List[Mapping[str, Any]]] = None,
+    requested_predicates: Optional[List[Mapping[str, Any]]] = None,
+    non_revoked: Optional[Mapping[str, int]] = None,
+    cred_rev_id: Optional[str] = None,
+):
+    """Present a credential using present proof v2 (indy)."""


Maybe worth noting that this implementation comes from the acapy_controller.protocols package (I'm guessing it does) -- with only difference being request_payload = _presentation_request_payload(holder_pres_ex)?

ff137 · 2026-01-07T11:55:57Z

scenarios/examples/util.py

    raise Exception("Error unable to upgrade wallet type to askar-anoncreds")


 # anoncreds utilities:


This _presentation_request_payload is duplicated in scenarios/examples/presenting_revoked_credential/example.py and kanon_issuance_and_presentation/example.py, so those can just import from here, and then # anoncreds utilities: can be moved to before the next def anoncreds_presentation_summary block, so it's clear _presentation_request_payload is not just for anoncreds

ff137 · 2026-01-26T12:25:25Z

@sonivijayk please note this PR will need some finishing touches before it can be merged (mainly DCO sign offs).

Vijay Soni and others added 10 commits January 3, 2026 13:44

fixed duplicate present-proof v2 webhook

8f6584b

Signed-off-by: Vijay Soni <[email protected]>

chore: Lower scenario test logging to info level (openwallet-foundati…

ddf5dd0

…on#4000) Signed-off-by: jamshale <[email protected]>

feat: Only log failing scenarios (openwallet-foundation#4005)

7354432

* feat: Only log failing scenarios Signed-off-by: jamshale <[email protected]> * fix: Remove silent mode Signed-off-by: jamshale <[email protected]> --------- Signed-off-by: jamshale <[email protected]>

fixed sonar issues

42d8570

Signed-off-by: Vijay Soni <[email protected]>

fixed linting err

2b53895

Signed-off-by: Vijay Soni <[email protected]>

fixed test errors

33a7849

Signed-off-by: Vijay Soni <[email protected]>

sonivijayk force-pushed the sonivijayk/fix/issue-3802-presentation-xchg-webhook-dedup branch from c7b95f1 to 33a7849 Compare January 3, 2026 19:46

Merge branch 'main' into sonivijayk/fix/issue-3802-presentation-xchg-…

15d3346

…webhook-dedup

jamshale requested review from Copilot, esune and ff137 January 6, 2026 17:13

Copilot started reviewing on behalf of jamshale January 6, 2026 17:14 View session

Copilot AI reviewed Jan 6, 2026

View reviewed changes

Merge branch 'main' into sonivijayk/fix/issue-3802-presentation-xchg-…

eb8d2ba

…webhook-dedup

ff137 reviewed Jan 7, 2026

View reviewed changes

-) -> Optional[Dict[str, Any]]:
+) -> Optional[Dict[str, Any]]:
+    """Extract presentation request payload from a presentation exchange record.
+    This helper prefers the `by_format.pres_request` field when available and
+    falls back to the legacy `pres_request` field. The legacy value may be a
+    plain dict or a model instance; in the latter case it is converted to a
+    dict representation using alias field names.
+    Args:
+        presentation: The v2.0 presentation exchange record containing the
+            formatted and/or legacy presentation request data.
+    Returns:
+        A dictionary representation of the presentation request payload, or
+        None if no request is available on the record.
+    """

		raise Exception("Error unable to upgrade wallet type to askar-anoncreds")


		# anoncreds utilities:

fixed duplicate present-proof v2 webhook #3998

Are you sure you want to change the base?

fixed duplicate present-proof v2 webhook #3998

Uh oh!

Conversation

sonivijayk commented Dec 23, 2025

Fix Summary

Fix Testing

Uh oh!

jamshale commented Jan 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes:

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jan 6, 2026

Quality Gate passed

Uh oh!

ff137 commented Jan 6, 2026

Uh oh!

ff137 left a comment

Choose a reason for hiding this comment

Uh oh!

ff137 Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

ff137 Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

ff137 Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

ff137 Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

ff137 Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

ff137 Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

ff137 commented Jan 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants