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

feat(core): ensure KEL, ACDC, contact timestamps are used instead of record creation time #860

Conversation

Sotatek-TungNguyen2a
Copy link
Collaborator

@Sotatek-TungNguyen2a Sotatek-TungNguyen2a commented Dec 10, 2024

Description

In many places where we create records we are letting the createdAt timestamp be Date.now() but it should match the KERIA times.

Checklist before requesting a review

Issue ticket number and link

Testing & Validation

  • This PR has been tested/validated in IOS, Android and browser.
  • The code has been tested locally with test coverage match expectations.
  • Added new Unit/Component testing (if relevant).

Security

  • No secrets are being committed (i.e. credentials, PII)
  • This PR does not have any significant security implications

Code Review

  • There is no unused functionality or blocks of commented out code (otherwise, please explain below)
  • In addition to this PR, all relevant documentation (e.g. Confluence) and architecture diagrams (e.g. Miro) were updated

@jimcase
Copy link
Contributor

jimcase commented Dec 10, 2024

In the UI CreareIdentifier.tsx, we have the method handleCreateIdentifier that is creating the Identify record like this:

const newIdentifier: IdentifierShortDetails = {
        id: identifier,
        displayName: identifierData.displayName,
        createdAtUTC: new Date().toISOString(),
        theme: selectedTheme,
        isPending: isPending,
      };

Should we set the createdAtUTC time to the one provided by keripy instead of new Date()? @iFergal

const identifierDetail = await this.props.signifyClient.identifiers().get(identifier);
new Date(identifierDetail.state.dt)

@iFergal
Copy link
Collaborator

iFergal commented Dec 10, 2024

@jimcase The state could be the datetime of a newer rotation event. But yes, this is what myself and Patrick mentioned this morning. I opened a PR just now on KERIA: WebOfTrust/keria#335

Copy link
Collaborator

@iFergal iFergal left a comment

Choose a reason for hiding this comment

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

Need to wait until next dev release of KERIA for latest change to get the real inception datetime.

src/core/agent/services/credentialService.ts Outdated Show resolved Hide resolved
@iFergal
Copy link
Collaborator

iFergal commented Dec 10, 2024

@jimcase I also misread your comment. Yes, except Bao's current PR is changing it so that it's created via callbacks so we can consolidate the two PRs. @Sotatek-BaoHoanga and @Sotatek-TungNguyen2a please coordinate here

@iFergal iFergal merged commit cd53be8 into develop Dec 13, 2024
1 check passed
@iFergal iFergal deleted the feat/DTIS-1417-using-keria-timestamps-instead-of-record-creation-time branch December 13, 2024 13:04
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.

3 participants