Skip to content

test(vdr): route ledger builders through indy-vdr - #1936

Open
malsomesh9 wants to merge 1 commit into
hyperledger-indy:mainfrom
malsomesh9:codex/fix-1644-indy-vdr
Open

test(vdr): route ledger builders through indy-vdr#1936
malsomesh9 wants to merge 1 commit into
hyperledger-indy:mainfrom
malsomesh9:codex/fix-1644-indy-vdr

Conversation

@malsomesh9

Copy link
Copy Markdown

Summary

Start the indy-sdk to indy-vdr migration by routing the test suite's ledger request builders through an indy-vdr compatibility layer.

Changes

  • add indy_node/test/indy_vdr_ledger.py as a focused compatibility module for ledger request construction
  • switch the test suite's indy.ledger request-builder imports over to the new compatibility layer
  • keep the remaining legacy signing helpers isolated in one place while the broader wallet/signing migration is still pending
  • add indy_vdr to the test dependencies

Verification

  • confirmed the routed indy-vdr builder functions exist in the upstream wrapper
  • ran python3 -m compileall indy_node/test successfully after the import migration

Notes

This is a substantial first slice of the migration, but not the full replacement of every remaining python3-indy usage yet. DID and signing helpers still need a follow-up migration path.

Related to #1644

Signed-off-by: malsomesh9 <malsomesh9@gmail.com>

@kukgini kukgini left a comment

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.

Review summary

I cross-checked this against the indy-vdr v0.4.1 Python wrapper source, and the core of the migration is solid: every routed builder exists upstream, parameter names and order match, all call sites in the test suite are compatible, and the negative tests (which corrupt requests via modify_field after building) are unaffected by indy-vdr's client-side validation. The import sweep is also complete — the changed files are exactly the set that imports indy.ledger builders.

There is one blocking bug, though: the reimplemented parse_get_schema_response drops seqNo, which breaks every schema→cred-def round-trip test at runtime.

🚫 Blocking: parse_get_schema_response must propagate seqNo

The reason these fixtures do a GET_SCHEMA round-trip instead of reusing the issuer_create_schema output is to obtain the schema's ledger seqNo (e.g. the schema_json fixtures in indy_node/test/claim_def/conftest.py). The failure chain with the current implementation:

  1. The parsed schema JSON has no seqNo (libindy's parse_get_schema_response returns {ver, id, name, version, attrNames, seqNo}).
  2. libindy's issuer_create_and_store_credential_def then falls back to the full schema id string ("<did>:2:<name>:<version>") as the cred-def schemaId, instead of the numeric seqNo.
  3. indy-vdr's build_cred_def_request does not error on a non-numeric schemaId — it silently builds ref: 0 (libindy_vdr/src/ledger/requests/cred_def.rs L26: data.schema_id.0.parse::<i32>().unwrap_or(0)).
  4. The node rejects the CLAIM_DEF because seqNo 0 doesn't reference the schema → RequestRejectedException.

Affected flows: claim_def/test_send_claim_def.py, claim_def/test_send_get_claim_def.py, api/test_claim_def_reply.py, auth_rule/auth_framework/claim_def.py, auth_rule/auth_framework/revoc_reg_def.py, and request_propagates/test_request_propagates.py.

The fix is small — in parse_get_schema_response in indy_node/test/indy_vdr_ledger.py:

seq_no = result.get("seqNo")
if seq_no is not None:
    data.setdefault("seqNo", seq_no)

Nice work on the signature mapping overall — build_get_attrib_request's keyword adaptation and the auth_type/txn_type aliasing in the auth-rule builders both line up exactly with the existing call sites in indy_node/test/helper.py.

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