From 0bb364ad159b01b76289d416a0a9e5f85a2c5fec Mon Sep 17 00:00:00 2001 From: Martin Saporiti Date: Mon, 8 Apr 2024 17:02:25 -0300 Subject: [PATCH 1/2] fix: publish state and improve performance when processing takes place --- internal/core/services/identity.go | 64 ++++++++++--------- internal/core/services/tests/identity_test.go | 51 ++++++++++++++- internal/gateways/publisher.go | 1 + internal/repositories/claims.go | 2 +- 4 files changed, 86 insertions(+), 32 deletions(-) diff --git a/internal/core/services/identity.go b/internal/core/services/identity.go index 39e876413..448aa4778 100644 --- a/internal/core/services/identity.go +++ b/internal/core/services/identity.go @@ -56,7 +56,7 @@ var ( // ErrAssigningMTPProof - represents an error in the identity metadata ErrAssigningMTPProof = errors.New("error assigning the MTP Proof from Auth Claim. If this identity has keyType=ETH you must to publish the state first") // ErrNoClaimsFoundToProcess - means that there are no claims to process - ErrNoClaimsFoundToProcess = errors.New("no MTP claims found to process") + ErrNoClaimsFoundToProcess = errors.New("no MTP or revoked claims found to process") ) type identity struct { @@ -327,42 +327,35 @@ func (i *identity) UpdateState(ctx context.Context, did w3c.DID) (*domain.Identi return fmt.Errorf("error getting the identifier last state: %w", err) } - var state *merkletree.Hash = nil - if previousState != nil { - state = previousState.TreeState().State - } - - lc, err := i.claimsRepository.GetAllByState(ctx, tx, &did, state) + // Get all mtp claims with state == nil + claimsAddedToTree, err := i.processClaims(ctx, tx, did, iTrees) if err != nil { - return fmt.Errorf("error getting the states: %w", err) + return err } - // Check if there are claims to process - if err := checkClaimsToAdd(lc); err != nil { + // Get all revocations with domain.RevPending status + updatedRevocations, err := i.revocationRepository.UpdateStatus(ctx, tx, &did) + if err != nil { + log.Error(ctx, "updating revocation status", "err", err) return err } - for i := range lc { - if lc[i].IdentityState == nil { - err = iTrees.AddClaim(ctx, &lc[i]) - if err != nil { - return err - } - } + log.Info(ctx, "updating revocation status", "revocations", len(updatedRevocations)) + + if len(updatedRevocations) == 0 && !claimsAddedToTree { + log.Info(ctx, "no claims or revocations found to process") + return ErrNoClaimsFoundToProcess } err = populateIdentityState(ctx, iTrees, newState, previousState) if err != nil { + log.Error(ctx, "populating identity state", "err", err) return err } err = i.update(ctx, tx, &did, *newState) if err != nil { - return err - } - - updatedRevocations, err := i.revocationRepository.UpdateStatus(ctx, tx, &did) - if err != nil { + log.Error(ctx, "updating claims", "err", err) return err } @@ -403,17 +396,28 @@ func (i *identity) UpdateState(ctx context.Context, did w3c.DID) (*domain.Identi return newState, err } -// checkClaimsToAdd checks if there are claims to process -// if the len of the claims is 0 or the len is 1 and the claim is the authBJJCredential then return an error -func checkClaimsToAdd(lc []domain.Claim) error { - if len(lc) == 0 { - return ErrNoClaimsFoundToProcess +func (i *identity) processClaims(ctx context.Context, tx pgx.Tx, did w3c.DID, iTrees *domain.IdentityMerkleTrees) (bool, error) { + lc, err := i.claimsRepository.GetAllByState(ctx, tx, &did, nil) + if err != nil { + return false, fmt.Errorf("error getting the states: %w", err) } - if len(lc) == 1 && lc[0].SchemaType == domain.AuthBJJCredentialTypeID { - return ErrNoClaimsFoundToProcess + claimsAddedToTree := false + if len(lc) > 0 { + log.Info(ctx, "adding claims to tree", "claims", len(lc)) + claimsAddedToTree = true } - return nil + + if claimsAddedToTree { + for i := range lc { + err = iTrees.AddClaim(ctx, &lc[i]) + if err != nil { + log.Error(ctx, "adding claim to tree", "err", err) + return false, err + } + } + } + return claimsAddedToTree, nil } func (i *identity) UpdateIdentityState(ctx context.Context, state *domain.IdentityState) error { diff --git a/internal/core/services/tests/identity_test.go b/internal/core/services/tests/identity_test.go index 41db65e54..c527bf507 100644 --- a/internal/core/services/tests/identity_test.go +++ b/internal/core/services/tests/identity_test.go @@ -98,7 +98,7 @@ func Test_identity_UpdateState(t *testing.T) { assert.NotNil(t, identityState.RevocationTreeRoot) }) - t.Run("should return an error after revoke a credential", func(t *testing.T) { + t.Run("should return success after revoke a MTP credential", func(t *testing.T) { ctx := context.Background() merklizedRootPosition := "index" claim, err := claimsService.Save(ctx, ports.NewCreateClaimRequest(did, schema, credentialSubject, @@ -121,7 +121,56 @@ func Test_identity_UpdateState(t *testing.T) { assert.NoError(t, claimsService.Revoke(ctx, *did, uint64(claim.RevNonce), "")) _, err = identityService.UpdateState(ctx, *did) + assert.NoError(t, err) + }) + + t.Run("should return pass after creating two credentials", func(t *testing.T) { + ctx := context.Background() + merklizedRootPosition := "index" + claimMTP, err := claimsService.Save(ctx, ports.NewCreateClaimRequest(did, schema, credentialSubject, + common.ToPointer(time.Now()), typeC, nil, nil, &merklizedRootPosition, + common.ToPointer(false), common.ToPointer(true), nil, false, + verifiable.SparseMerkleTreeProof, nil, nil, nil)) + + assert.NoError(t, err) + previousStateIdentity, _ := identityStateRepo.GetLatestStateByIdentifier(ctx, storage.Pgx, did) + identityState, err := identityService.UpdateState(ctx, *did) + assert.NoError(t, err) + assert.Equal(t, did.String(), identityState.Identifier) + assert.NotNil(t, identityState.State) + assert.Equal(t, domain.StatusCreated, identityState.Status) + assert.NotNil(t, identityState.StateID) + assert.Equal(t, previousStateIdentity.State, identityState.PreviousState) + assert.NotNil(t, identityState.RootOfRoots) + assert.NotNil(t, identityState.ClaimsTreeRoot) + assert.NotNil(t, identityState.RevocationTreeRoot) + + assert.NoError(t, claimsService.Revoke(ctx, *did, uint64(claimMTP.RevNonce), "")) + _, err = identityService.UpdateState(ctx, *did) + assert.NoError(t, err) + + claimSIG, err := claimsService.Save(ctx, ports.NewCreateClaimRequest(did, schema, credentialSubject, + common.ToPointer(time.Now()), typeC, nil, nil, &merklizedRootPosition, + common.ToPointer(true), common.ToPointer(false), nil, false, + verifiable.SparseMerkleTreeProof, nil, nil, nil)) + + assert.NoError(t, err) + _, err = identityService.UpdateState(ctx, *did) assert.Error(t, err) + + assert.NoError(t, claimsService.Revoke(ctx, *did, uint64(claimSIG.RevNonce), "")) + identityState, err = identityService.UpdateState(ctx, *did) + assert.NoError(t, err) + previousStateIdentity, err = identityStateRepo.GetLatestStateByIdentifier(ctx, storage.Pgx, did) + assert.NoError(t, err) + assert.Equal(t, did.String(), identityState.Identifier) + assert.NotNil(t, identityState.State) + assert.Equal(t, domain.StatusCreated, identityState.Status) + assert.NotNil(t, identityState.StateID) + assert.Equal(t, previousStateIdentity.State, identityState.PreviousState) + assert.NotNil(t, identityState.RootOfRoots) + assert.NotNil(t, identityState.ClaimsTreeRoot) + assert.NotNil(t, identityState.RevocationTreeRoot) }) t.Run("should get an error creating credential with sig proof", func(t *testing.T) { diff --git a/internal/gateways/publisher.go b/internal/gateways/publisher.go index 0ba60ded2..e4f854204 100644 --- a/internal/gateways/publisher.go +++ b/internal/gateways/publisher.go @@ -136,6 +136,7 @@ func (p *publisher) publishState(ctx context.Context, identifier *w3c.DID) (*dom txID, err := p.publishProof(ctx, identifier, *updatedState) if err != nil { + // TODO: Handle RHS status already published log.Error(ctx, "Error during publishing proof:", "err", err, "did", identifier.String()) updatedState.Status = domain.StatusFailed errUpdating := p.identityService.UpdateIdentityState(ctx, updatedState) diff --git a/internal/repositories/claims.go b/internal/repositories/claims.go index 6f8bad38e..e68a163a6 100644 --- a/internal/repositories/claims.go +++ b/internal/repositories/claims.go @@ -559,7 +559,7 @@ func (c *claims) GetAllByState(ctx context.Context, conn db.Querier, did *w3c.DI credential_status, core_claim FROM claims - WHERE issuer = $1 AND identity_state IS NULL AND identifier = issuer AND (mtp = true OR revoked = true) + WHERE issuer = $1 AND identity_state IS NULL AND identifier = issuer AND mtp = true `, did.String()) } else { rows, err = conn.Query(ctx, ` From a65ef21dbade98f9e5c65d15fb7c8b37bd53276b Mon Sep 17 00:00:00 2001 From: Martin Saporiti Date: Thu, 11 Apr 2024 10:32:47 -0300 Subject: [PATCH 2/2] chore: improve code --- internal/core/services/identity.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/core/services/identity.go b/internal/core/services/identity.go index 448aa4778..ecbccad37 100644 --- a/internal/core/services/identity.go +++ b/internal/core/services/identity.go @@ -408,15 +408,15 @@ func (i *identity) processClaims(ctx context.Context, tx pgx.Tx, did w3c.DID, iT claimsAddedToTree = true } - if claimsAddedToTree { - for i := range lc { - err = iTrees.AddClaim(ctx, &lc[i]) - if err != nil { - log.Error(ctx, "adding claim to tree", "err", err) - return false, err - } + for i := range lc { + err = iTrees.AddClaim(ctx, &lc[i]) + if err != nil { + log.Error(ctx, "adding claim to tree", "err", err) + return false, err } + } + return claimsAddedToTree, nil }