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

Greg/967/stake distribution artifact #980

Merged
merged 9 commits into from Jun 20, 2023

Conversation

ghubertpalo
Copy link
Collaborator

@ghubertpalo ghubertpalo commented Jun 14, 2023

Content

This PR adds missing fields in the StakeDistribution signed entities. It will ease the forging of the signature on the client.

Pre-submit checklist

  • Branch
    • Crates versions are updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • No clippy warnings in the CI
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

Comments

This is code refactoring, it will not change the user experience.

Issue(s)

Closes #967

@ghubertpalo ghubertpalo force-pushed the greg/967/stake_distribution_artifact branch 2 times, most recently from 0810db0 to 69a7170 Compare June 16, 2023 09:20
@ghubertpalo ghubertpalo temporarily deployed to testing-preview June 16, 2023 09:34 — with GitHub Actions Inactive
@ghubertpalo ghubertpalo force-pushed the greg/967/stake_distribution_artifact branch from 5233ca2 to d3b42bb Compare June 16, 2023 13:31
@ghubertpalo ghubertpalo temporarily deployed to testing-preview June 16, 2023 13:52 — with GitHub Actions Inactive
@ghubertpalo ghubertpalo temporarily deployed to testing-preview June 16, 2023 14:40 — with GitHub Actions Inactive
Copy link
Member

@jpraynaud jpraynaud left a comment

Choose a reason for hiding this comment

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

Few comments but looks good otherwise!

mithril-aggregator/src/database/migration.rs Outdated Show resolved Hide resolved
mithril-aggregator/src/database/migration.rs Outdated Show resolved Hide resolved
mithril-common/src/entities/mithril_stake_distribution.rs Outdated Show resolved Hide resolved
@@ -62,6 +62,7 @@ export default function MithrilStakeDistributionsList(props) {
<Card.Title>{mithrilStakeDistribution.hash}</Card.Title>
<ListGroup variant="flush" className="data-list-group">
<ListGroup.Item>Epoch: {mithrilStakeDistribution.epoch}</ListGroup.Item>
<ListGroup.Item>Created: {mithrilStakeDistribution.created_at}</ListGroup.Item>
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can add this in a separate PR if this is a breaking change for the explorer?
This may break the pre-release and release environments until they run this PR.

Copy link
Collaborator

@Alenar Alenar Jun 19, 2023

Choose a reason for hiding this comment

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

Alternatively you can show this field only if it exist with something like this (note the usage of the date function to show the date to the user using his timezone & format):

{mithrilStakeDistribution.created_at &&
  <ListGroup.Item>Created: {new Date(mithrilStakeDistribution.created_at).toLocaleString()}</ListGroup.Item>
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This commit has been isolated into a new branch so we can deploy the explorer on its own.

openapi.yaml Outdated Show resolved Hide resolved
@ghubertpalo ghubertpalo force-pushed the greg/967/stake_distribution_artifact branch from 5a3f582 to d3b42bb Compare June 19, 2023 08:25
@ghubertpalo ghubertpalo temporarily deployed to testing-preview June 19, 2023 08:34 — with GitHub Actions Inactive
Copy link
Member

@jpraynaud jpraynaud left a comment

Choose a reason for hiding this comment

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

LGTM

@ghubertpalo ghubertpalo force-pushed the greg/967/stake_distribution_artifact branch from fee9669 to 96d8f2e Compare June 19, 2023 13:02
@ghubertpalo ghubertpalo temporarily deployed to testing-preview June 19, 2023 13:10 — with GitHub Actions Inactive
@ghubertpalo ghubertpalo merged commit 699c534 into main Jun 20, 2023
23 checks passed
@ghubertpalo ghubertpalo deleted the greg/967/stake_distribution_artifact branch June 20, 2023 07:32
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.

Refactor MithrilStakeDistribution entity
3 participants