Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additional endpoints to get revocation details and fix "published" status #1783

Merged
merged 6 commits into from
Jun 9, 2022
Merged

Additional endpoints to get revocation details and fix "published" status #1783

merged 6 commits into from
Jun 9, 2022

Conversation

ianco
Copy link
Contributor

@ianco ianco commented May 24, 2022

Signed-off-by: Ian Costanzo [email protected]

This PR is good to go:

  • added a couple of endpoints to get more detailed revocation info out of the wallet and leger
  • added an endpoint to post a ledger correcting entry (based on script from @andrewwhitehead )
  • fixed askar credential revocation state
  • added a new parameter to alice/faber --taa-accept to enforce TAA acceptance when required
  • added some integration tests and some docs - tested using indy and askar wallets

@ianco ianco added the 0.7.4 label May 24, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jun 2, 2022

Codecov Report

Merging #1783 (a2a5db0) into main (e7fd7e5) will decrease coverage by 0.35%.
The diff coverage is 33.50%.

@@            Coverage Diff             @@
##             main    #1783      +/-   ##
==========================================
- Coverage   94.11%   93.76%   -0.36%     
==========================================
  Files         533      534       +1     
  Lines       33640    33829     +189     
==========================================
+ Hits        31661    31719      +58     
- Misses       1979     2110     +131     

@ianco ianco marked this pull request as ready for review June 2, 2022 23:12
LOGGER.info(
">>> pre-pending -endian character to TAA acceptance text"
)
accept_input["text"] = "\ufeff" + accept_input["text"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ran into this issue specifically on the Sovrin builder net, not sure if it would happen on the other networks.

This function can be used if there were previous revocation errors (i.e. the
credential revocation was successfully written to the wallet but the ledger write
failed.)
"""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to keep this script relatively self-contained (no dependencies on other aca-py code) to make it easier to port if anyone else needs to do this (any other agents), I expect the revocation corruption problem will be fairly common.


@T003-TAA @taa_required
Scenario Outline: Fail to publish revoked credential using a ledger with TAA required, and fix the ledger
Given we have "2" agents
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the scenario that we ran into with LSBC - a failed ledger transaction corrupted the revocation registry and caused future revocations to fail. The "posts a correction to the ledger" step invokes the new aca-py endpoint to fix the ledger.

)
await issuer_cr_rec.set_state(
txn, IssuerCredRevRecord.STATE_REVOKED
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved this code per @andrewwhitehead comment

async with ledger:
try:
taa_info = await ledger.get_txn_author_agreement()
if not taa_info["taa_required"]:
raise web.HTTPBadRequest(
reason=f"Ledger {ledger.pool_name} TAA not available"
)
LOGGER.info(f"TAA on ledger: {taa_info}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
LOGGER.info(f"TAA on ledger: {taa_info}")
LOGGER.info("TAA on ledger: %r", taa_info)


LOGGER.debug(">>> apply_ledger_update = %s", apply_ledger_update)
if apply_ledger_update:
ledger = session.inject(BaseLedger)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs to be inject_or if you want to handle a missing ledger

async def fetch_txns(genesis_txns, registry_id):
"""Fetch tails file and revocation registry information."""

vdr_module = importlib.import_module("indy_vdr")
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can handle an import error here if the dependencies are missing?

ianco added 2 commits June 3, 2022 10:55
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
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.

4 participants