Anoncreds - Cred Def and Revocation Endorsement

[jamshale]

• Implements anoncreds endorsement of cred def's and revocation objects
• enables the integration tests both manual and auto endorsement transactions

For the anoncreds transaction objects the previous implementation changed the structure of the objects. I really didn't like this so I changed it in the indy ledger module with if statements.

I changed the optional[dict] params in anoncreds to be an empty dict by default to avoid null checking in a lot of places.

Note: The only thing not completed for single tenant is publishing revocations. I want to do this in another task/PR. I created an integration test for anoncreds that stops just prior to publishing revocations.

[ianco]

FYI I've noticed some integration tests are now failing, for example:

AGENT_PORT_OVERRIDE=8030 ./run_bdd -t @RFC0586 -t @WalletType_Askar_AnonCreds

(Note that these aren't tagged as @GHA so they aren't included in the set of tests that runs on a PR. Possibly we should add this tag to these tests ... they haven't been included so far as the anoncreds/endorsement is still in progress)

[jamshale]

@ianco Ya I had the integration tests working but then I did some refactoring and broke them somehow. Was going to try and look at why this happened today.

[jamshale]

FYI I've noticed some integration tests are now failing, for example:

AGENT_PORT_OVERRIDE=8030 ./run_bdd -t @RFC0586 -t @WalletType_Askar_AnonCreds

(Note that these aren't tagged as @GHA so they aren't included in the set of tests that runs on a PR. Possibly we should add this tag to these tests ... they haven't been included so far as the anoncreds/endorsement is still in progress)

I know these had been working. I'll work on fixing them and then I will add them to run on a PR.

[jamshale]

Updated description. Ready for review. Integration tests should pass.

[jamshale]

Hmm. Some integration tests are failing. I'll have to look into it.

[ianco]

Hmm. Some integration tests are failing. I'll have to look into it.

Looks like you're trying to issue the credential before the revocation registry is active. Try just running alice/faber with the --revocation flag (and anoncreds-askar wallet) and try to issue the credential manually.

[jamshale]

Hmm. Some integration tests are failing. I'll have to look into it.

Looks like you're trying to issue the credential before the revocation registry is active. Try just running alice/faber with the --revocation flag (and anoncreds-askar wallet) and try to issue the credential manually.

I think I was focusing to much on the endorsement tests and might have broke creating the registry for non-endorsement. Good thing we have so many integration tests. Hopefully it's minor. Will ping you when it's fixed.

[jamshale]

@ianco I did have a logic issue with the various configs/params. Tests are passing now.

[jamshale]

One integration test is failing, even though it passed one time. I think it's taking too long to create the revocation registry. Going to try and fix it.

[ianco]

One integration test is failing, even though it passed one time. I think it's taking too long to create the revocation registry. Going to try and fix it.

It's possibly creating the revocation registry but not activating it:

      Traceback (most recent call last):
        File "/home/aries/.local/lib/python3.9/site-packages/behave/model.py", line 1329, in run
          match.run(runner.context)
        File "/home/aries/.local/lib/python3.9/site-packages/behave/matchers.py", line 98, in run
          self.func(context, *args, **kwargs)
        File "features/steps/0586-sign-transaction.py", line 421, in step_impl
          assert len(rev_regs["rev_reg_ids"]) >= 1
      AssertionError

(There should always be exactly one Active revocation registry per cred def, even though there may be more than one created)

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

[ianco]

LGTM. I did a quick scan of the code and ran the demo. Looks like all the tests are passing. Good job!

[swcurran]

W00t! Nicely done!

[ff137]

FYI, default empty dicts in args are 'dangerous' in python:
https://stackoverflow.com/questions/26320899/why-is-the-empty-dictionary-a-dangerous-default-value-in-python

Pylint warns about it, so ruff should probably pick up on this? @dbluhm (because you know about the ruff config)

[dbluhm]

Really good point. Looks like this would be covered by rule B006, which we don't currently have activated.

@jamshale it would be great to see a follow up PR to add checks for this rule and fix this argument.

[jamshale]

I'll have another PR for the publish revocations and I'll do that.

[jamshale]

I had no idea python did that. Seems really odd a default function argument wouldn't be scoped to the function.

[dbluhm]

I have personal experience with this foot-gun lol. Debugging without the context of the "static" evaluation of the default arguments was a nightmare. The Ruff check will be good to help prevent future headaches.

[ff137]

required is not a metadata field -- should be passed as required = False, metadata=...
I'll fix in my deprecation warnings PR (which had merge conflicts here)