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

doc: update PFM spec #293

Closed
wants to merge 1 commit into from
Closed

Conversation

renaynay
Copy link
Member

I mostly made this PR to track some misunderstandings I had with the spec for using PFM.

@@ -113,15 +113,15 @@ if err != nil {
}

// query for account information necessary to sign a valid tx
err = keyringSigner.QueryAccount(ctx, grpcClientConn)
err = keyringSigner.QueryAccountNumber(ctx, grpcClientConn)
Copy link
Member Author

Choose a reason for hiding this comment

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

Is this the method we should be using?

Copy link
Member

Choose a reason for hiding this comment

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

this brings up a really good point. This method appears to be vestigial, and should likely not be exported. Instead, it should be private and only called during the keyringSigner's BroadcastTx method.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

For the moment though, is this a necessary component to building and submitting a PFD?

@adlerjohn adlerjohn added the documentation Improvements or additions to documentation label Apr 13, 2022
@liamsi
Copy link
Member

liamsi commented May 10, 2022

what is the status of this PR?

@evan-forbes
Copy link
Member

I think we can close, as I believe we covered this issue synchronously but will defer to @renaynay

@renaynay
Copy link
Member Author

We can close this. We should probably provide docs on PFD in general for @YazzyYaz to put in docs.celestia.org though!

@liamsi liamsi closed this May 11, 2022
@renaynay renaynay deleted the update-pfm-docs branch May 11, 2022 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants