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: add oracle upgrade related genesis #583

Merged
merged 6 commits into from
Jan 4, 2023

Conversation

audtlr24
Copy link
Contributor

closes #581 .

@audtlr24 audtlr24 added this to the v2.1.0-beta milestone Dec 29, 2022
@audtlr24 audtlr24 self-assigned this Dec 29, 2022
@audtlr24 audtlr24 changed the title feat : add oracle upgrade related genesis feat: add oracle upgrade related genesis Dec 29, 2022
Copy link
Contributor

@0xHansLee 0xHansLee left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link

@inchori inchori left a comment

Choose a reason for hiding this comment

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

Gooood!

}
}

if m.OracleUpgradeInfo != nil {
Copy link

Choose a reason for hiding this comment

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

Does the OracleUpgradeInfo in GenesisState should be always not nil?

Copy link

Choose a reason for hiding this comment

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

Ah.. I understand it. Please forgot this comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we follow the specs in this link, I think we should check it like @inchori said.

Copy link
Contributor

Choose a reason for hiding this comment

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

right!

Copy link
Contributor

Choose a reason for hiding this comment

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

The OracleUpgradeInfo can be nil if there is no upgrade.
It seems no problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

Aha, I was confused about how far the upgrade action could go.
If the upgrade command cannot be performed as long as the upgrade action does not occur, now is OK.
However, even if there is no upgrade operation, if the upgrade command is available (i.e. OracleShareKey is lost), we must set uniqueID in params and upgradeInfo when launching DEP.

As @H4NLee said before, if oracle can't use this address when the key is lost, no problem.

Copy link
Contributor

@gyuguen gyuguen left a comment

Choose a reason for hiding this comment

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

LGTM

}
}

if m.OracleUpgradeInfo != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

If we follow the specs in this link, I think we should check it like @inchori said.

@audtlr24 audtlr24 merged commit 5b3d9a4 into ft/571/upgrade-oracle-tx Jan 4, 2023
@audtlr24 audtlr24 deleted the ft/581/oracle-upgrade-genesis branch January 4, 2023 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants