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 CLI register-oracle #17

Merged
merged 7 commits into from
Dec 8, 2022
Merged

feat: Add CLI register-oracle #17

merged 7 commits into from
Dec 8, 2022

Conversation

0xHansLee
Copy link
Contributor

close #2

@0xHansLee 0xHansLee added this to the v2.1.0-beta milestone Dec 6, 2022
@0xHansLee 0xHansLee self-assigned this Dec 6, 2022
@0xHansLee 0xHansLee marked this pull request as draft December 6, 2022 04:30
@0xHansLee 0xHansLee marked this pull request as ready for review December 6, 2022 05:50
Comment on lines 83 to 119
// subscribe approval of oracle registration and handle it
client, err := rpchttp.New(conf.Panacea.RPCAddr, "/websocket")
if err != nil {
return err
}

if err := client.Start(); err != nil {
return err
}
defer func() {
_ = client.Stop()
}()

event := oracleevent.NewApproveOracleRegistrationEvent()

ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
defer cancel()

txs, err := client.Subscribe(ctx, "", event.GetEventQuery())
if err != nil {
return err
}

errChan := make(chan error, 1)

for tx := range txs {
errChan <- event.EventHandler(tx)
}

err = <-errChan
if err != nil {
log.Infof("Error occurs while getting shared oracle private key. Please retrieve it via get-oracle-key cmd: %v", err)
return err
} else {
log.Infof("oracle private key is successfully shared. You can start oracle now!")
return nil
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If MsgRegisterOracle succeed, subscribe MsgApproveOracleRegistration and handle it in a synchronous way.

Copy link
Contributor

@audtlr24 audtlr24 left a comment

Choose a reason for hiding this comment

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

Thanks 👍 lgtm!
But I have one question here.

cmd/oracled/cmd/register_oracle.go Show resolved Hide resolved
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.

Good

return err
}
defer func() {
_ = client.Stop()
Copy link
Contributor

Choose a reason for hiding this comment

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

How about printing a log when error occurs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add it. Thx!

cmd/oracled/cmd/register_oracle.go Show resolved Hide resolved
Copy link
Contributor

@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.

LGTM 👍
I left a one question.

}

func NewApproveOracleRegistrationEvent(s event.Reactor) ApproveOracleRegistrationEvent {
return ApproveOracleRegistrationEvent{s}
func NewApproveOracleRegistrationEvent() ApproveOracleRegistrationEvent {
Copy link
Contributor

Choose a reason for hiding this comment

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

I have a question. Why remove the reactor for now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For this PR, I don't want to create a service for reactor, which has many things in it.
This ApproveOracleRegistrationEvent don't need all that resources in service.
I've been thinking about how to make this ApproveOracleRegistrationEvent better. If you have any idea, please feel free to leave your thoughts.

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand it! I'll also thinking about how to make it better. Thanks!

Copy link
Contributor

@youngjoon-lee youngjoon-lee left a comment

Choose a reason for hiding this comment

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

good

@0xHansLee 0xHansLee merged commit 1cec29d into main Dec 8, 2022
@0xHansLee 0xHansLee deleted the ft/2/register-oracle branch December 8, 2022 04:33
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.

Add CLI: register-oracle
5 participants