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.

Sign up for GitHub

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

docs: add QGB verify command docs #1287

Merged
merged 7 commits into from
Jan 31, 2023
Merged

docs: add QGB verify command docs #1287

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
601461c
docs: add QGB verify command docs
rach-id Jan 24, 2023
f10c8e2
Update x/qgb/README.md
rach-id Jan 24, 2023
9245044
Update x/qgb/README.md
rach-id Jan 24, 2023
dc61ae6
chore: msg -> blob
rach-id Jan 24, 2023
3106431
chore: msg -> blob
rach-id Jan 24, 2023
438bcc1
Update x/qgb/README.md
rach-id Jan 24, 2023
de21d45
Update x/qgb/README.md
rach-id Jan 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions x/qgb/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,42 @@ if k.CheckLatestAttestationNonce(ctx) && k.GetLatestAttestationNonce(ctx)+1 != n
}
```

## Verification command

The QGB verification command is part of the `celestia-appd` binary. It allows the user to verify that a set of shares has been posted to a specific QGB contract.

```shell
$ celestia-appd verify --help
Comment on lines +125 to +126
Copy link
Collaborator

Choose a reason for hiding this comment

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

[non-blocking] This code snippet may become out of date with the true --help docs so instead of copy + pasting the entire help output, I think it may be preferable to include the text that isn't the code snippet and at the bottom say something like:

See the following command for more details

celestia-appd verify --help

and then ensure that these docs make it into the appropriate --help docs

Copy link
Member Author

@rach-id rach-id Jan 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

What do you think is better for docs, providing the commands but without an output, or the commands + their output (that might be outdated after some time)?

Copy link
Contributor

Choose a reason for hiding this comment

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

I personally think the latter is better, and we should just update the docs to reflect the new flags/arguments later

Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you think is better for docs, providing the commands but without an output, or the commands + their output (that might be outdated after some time)?

In this scenario: providing the commands but without an output. I think this because the goal of the README is to give developers a high level overview of the QGB module. Developers aren't necessarily users of the QGB module CLI. Users of the QGB CLI should be able to get information on CLI flags via --help and I expect them to try that before finding this README.

Also (repeated from above): the command output becomes a maintenance burden but I defer to you @sweexordious

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess we can leave it as it is for now, with the help output. And, if it becomes a burden later, we can remove it.
Thanks guys for your help 👍 👍


Verifies that a transaction hash, a range of shares, or a blob referenced by its transaction hash were committed to by the QGB contract

Usage:
celestia-appd verify [command]

Available Commands:
blob Verifies that a blob, referenced by its transaction hash, in hex format, has been committed to by the QGB contract. Only supports one blob for now
shares Verifies that a range of shares has been committed to by the QGB contract
tx Verifies that a transaction hash, in hex format, has been committed to by the QGB contract

Flags:
-h, --help help for verify

Global Flags:
--home string directory for config and data (default "/home/midnight/.celestia-app")
--log_format string The logging format (json|plain) (default "plain")
--log_level string The logging level (trace|debug|info|warn|error|fatal|panic) (default "info")
--trace print out full stack trace on errors

Use "celestia-appd verify [command] --help" for more information about a command.

```

It currently supports three sub-commands:

- `blob`: Takes a transaction hash, in hex format, and verifies that the blob paid for by the transaction has been committed to by the QGB contract. It only supports one blob for now.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this specify that a prefix is needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Which prefix? A transaction hash in hex format is something like: 7826D5310CD007F82B918359D74A853B4ECB6633E9F231C5477B7F21D68D642D if I am not missing something

Copy link
Contributor

Choose a reason for hiding this comment

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

Nvm, I was wrong. Maybe we should still specify that no prefix is required

evan-forbes marked this conversation as resolved.
Show resolved Hide resolved
- `shares`: Takes a range of shares and a height, and verifies that these shares have been committed to by the QGB contract.
- `tx`: Takes a transaction hash, in hex format, and verifies that it has been committed to by the QGB contract.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above


## Useful links

The smart contract implementation is in [quantum-gravity-bridge](https://github.com/celestiaorg/quantum-gravity-bridge/).
Expand Down