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

Add section for suave-std api #92

Merged
merged 6 commits into from
Mar 14, 2024
Merged

Conversation

ferranbt
Copy link
Contributor

This PR does two things:

  • It introduces another docusaurus/plugin-content-docs to host suave-std documentation.
  • It uses the suave-std docs generator tool to create an API reference. Note that right now the content was moved manually. The idea is to have an automation tool to create sync up PRs as soon as the docs on suave-std change.

This PR is dependant on flashbots/suave-std#62 being merged first. This is a reference to see how the markdown files look like.

Copy link

vercel bot commented Mar 11, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
suave-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 14, 2024 8:26am

@metachris
Copy link
Contributor

metachris commented Mar 12, 2024

Nice! Perhaps adding it into the existing docs would be better than adding a whole new docs section 🤔

The formatting looks a bit plain, but we can improve that over time.

@ferranbt
Copy link
Contributor Author

Nice! Perhaps adding it into the existing docs would be better than adding a whole new docs section

I am expecting that section to grow over time with more examples or specific docs on Forge integration. We can put it into docs and move it out if it once it grows.

The formatting looks a bit plain, but we can improve that over time.

Agree, that is just the default format.

Copy link
Member

@zeroXbrock zeroXbrock left a comment

Choose a reason for hiding this comment

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

yes. so happy to see this

Copy link
Collaborator

@andytudhope andytudhope left a comment

Choose a reason for hiding this comment

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

Most of my comments are notes to myself which will be useful when we get this merged.

The think I'd really like you to fix @ferranbt is the line refs in all the links - those all need to point at the actual functions, and I think they should point to the line where the function is defined, not where it ends, and not a random line within it. If you can do that, I will get the rest of the things in order an think about how this fits into my plans for the docs this week.

Really awesome work though, thank you so much 🙏


## Functions

### [encodeRLP](https://github.com/flashbots/suave-std/tree/main/src/Transactions.sol#L109)
Copy link
Collaborator

Choose a reason for hiding this comment

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

These line numbers are all incorrect - can you fix them with the automatic tool?

I think that the Structs section should be moved to the top of this file so that it is clear what the difference between 155 and 1559, as well as transactions and "Request transactions" is.

I think that we should add at least one more line to the top of each file to explain why this library is useful/what often-used function it fulfills for suapp devs.


## Functions

### [emitOffchainLogs](https://github.com/flashbots/suave-std/tree/main/src/Suapp.sol#L9)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Better to reference the line where the function begins imo.


## Functions

### [randomUint8](https://github.com/flashbots/suave-std/tree/main/src/Random.sol#L10)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Line references seem to be incorrect.


## Functions

### [sendBundle](https://github.com/flashbots/suave-std/tree/main/src/protocols/Bundle.sol#L25)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Incorrect line refs again


### [confidentialRetrieve](https://github.com/flashbots/suave-std/tree/main/src/suavelib/Suave.sol#L197)

Retrieves data from the confidential store. Also mandates the caller's presence in the `AllowedPeekers` list.
Copy link
Collaborator

Choose a reason for hiding this comment

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

strange character here? '


Output:

- "b": Whether execution is off- or on-chain
Copy link
Collaborator

Choose a reason for hiding this comment

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

unclear - is this a boolean. Better to be explicit in our public docs.


# Suave-std

Suave-std is a set of utility libraries to build Suapps.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably needs more detail here if we merge this.

@andytudhope andytudhope merged commit 3a94e46 into main Mar 14, 2024
3 checks passed
@andytudhope andytudhope deleted the feature/suave-std-api-refs branch March 14, 2024 08: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.

4 participants