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: switching to axum, adding swagger UI #84

Merged
merged 2 commits into from
Jul 20, 2024
Merged

Conversation

distractedm1nd
Copy link
Contributor

Part of #65

WIP. Still quite a few issues with the API

But now we have Swagger UI:
IMAGE 2024-07-19 20:03:19

Copy link

coderabbitai bot commented Jul 19, 2024

Walkthrough

The recent updates transition the web server from the Actix to the Axum framework, enhancing routing and middleware capabilities. Key database methods related to epoch management have been removed to simplify the interface. New dependencies have been introduced for better API documentation and CORS handling, reflecting a focus on building robust web applications. Additionally, improvements in code readability and adjustments to epoch operation management signify a broader restructuring of the project's architecture and functionality.

Changes

File(s) Change Summary
Cargo.toml Added new dependencies: axum (0.6), tower-http (0.4, cors feature), utoipa (3.3, axum_extras), and utoipa-swagger-ui (3.1, axum).
src/node_types/sequencer.rs, src/storage.rs, src/utils.rs Updated handling of epoch operation management; removed several database methods; simplified logic for is_not_revoked function.
src/webserver.rs Transitioned to Axum framework; redefined routing and error handling; incorporated OpenAPI documentation annotations; refactored request handling functions.
src/error.rs, src/node_types/mod.rs Enhanced error handling by modifying GeneralError enum to include structured errors; removed test module for signature verification.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant WebServer
    participant Database

    Client->>WebServer: Send request (e.g., /update-entry)
    WebServer->>Database: Execute update operation
    Database-->>WebServer: Return operation result
    WebServer-->>Client: Send response with result
Loading
sequenceDiagram
    participant OldServer
    participant Client
    participant OldDatabase

    Client->>OldServer: Send request (e.g., /update-entry)
    OldServer->>OldDatabase: Execute update operation
    OldDatabase-->>OldServer: Return operation result
    OldServer-->>Client: Send response with result
Loading

Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between aa9a8fb and 77dbbc0.

Files selected for processing (7)
  • src/da/mod.rs (3 hunks)
  • src/error.rs (2 hunks)
  • src/node_types/mod.rs (1 hunks)
  • src/node_types/sequencer.rs (12 hunks)
  • src/storage.rs (9 hunks)
  • src/utils.rs (4 hunks)
  • src/webserver.rs (2 hunks)
Files skipped from review due to trivial changes (1)
  • src/node_types/mod.rs
Files skipped from review as they are similar to previous changes (1)
  • src/storage.rs
Additional comments not posted (19)
src/da/mod.rs (2)

34-36: LGTM!

The renaming of the method to get_plaintext is approved. The functionality remains the same and the new name improves clarity.


Line range hint 25-33:
LGTM! But verify the usage of the new trait and method names.

The changes to the trait implementation and method names are approved.

However, ensure that all references to the Signable trait and get_content_to_sign method are updated to SignedContent and get_plaintext, respectively.

Verification successful

Verification successful!

The references to the Signable trait and get_content_to_sign method have been correctly updated to SignedContent and get_plaintext, respectively, throughout the codebase.

  • SignedContent references found in:

    • src/webserver.rs
    • src/utils.rs
    • src/da/mod.rs
  • get_plaintext references found in:

    • src/webserver.rs
    • src/utils.rs
    • src/da/mod.rs
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to the `Signable` trait and `get_content_to_sign` method are updated.

# Test: Search for references to the `Signable` trait. Expect: No occurrences.
rg --type rust 'Signable'

# Test: Search for references to the `get_content_to_sign` method. Expect: No occurrences.
rg --type rust 'get_content_to_sign'

Length of output: 62


Script:

#!/bin/bash
# Description: Verify the presence of the new trait `SignedContent` and the method `get_plaintext`.

# Test: Search for references to the `SignedContent` trait. Expect: Multiple occurrences.
rg --type rust 'SignedContent'

# Test: Search for references to the `get_plaintext` method. Expect: Multiple occurrences.
rg --type rust 'get_plaintext'

Length of output: 605

src/error.rs (1)

1-1: LGTM! But verify the usage of the InvalidSignature variant.

The changes to the InvalidSignature variant are approved.

However, ensure that all references to the InvalidSignature variant are updated to handle the new SignatureError context.

Also applies to: 39-40

Verification successful

Let's inspect the context around the usage of InvalidSignature in src/utils.rs to confirm it handles SignatureError correctly.


LGTM! The usage of the InvalidSignature variant correctly handles the SignatureError context.

  • The InvalidSignature variant in GeneralError is properly defined to wrap SignatureError.
  • The usage in src/utils.rs correctly handles SignatureError by wrapping it in GeneralError::InvalidSignature and propagating the error.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to the `InvalidSignature` variant are updated to handle the new `SignatureError` context.

# Test: Search for references to the `InvalidSignature` variant. Expect: All occurrences handle `SignatureError`.
rg --type rust 'InvalidSignature'

Length of output: 170


Script:

#!/bin/bash
# Display the context around the usage of `InvalidSignature` in `src/utils.rs`
rg --type rust 'InvalidSignature' -A 5 -B 5 src/utils.rs

Length of output: 332

src/webserver.rs (7)

145-168: LGTM! But verify the usage of the get_hashchain function.

The changes to the get_hashchain function are approved.

However, ensure that all references to the get_hashchain function are correctly used in the codebase.


97-111: LGTM! But verify the usage of the new routing setup.

The changes to the start method of the WebServer struct are approved.

However, ensure that all references to the new routing setup are correctly used in the codebase.

Verification successful

LGTM! But verify the usage of the new routing setup.

The changes to the start method of the WebServer struct are approved.

The routing setup is correctly defined and documented within src/webserver.rs. No further references to the routes were found elsewhere in the codebase.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to the new routing setup are correctly used.

# Test: Search for references to the new routing setup. Expect: Correct usage.
rg --type rust 'Router::new'

Length of output: 77


Script:

#!/bin/bash
# Description: Verify all references to the routes defined in the new routing setup are correctly used.

# Test: Search for references to the new routes. Expect: Correct usage.
rg --type rust '/update-entry' -A 5
rg --type rust '/get-hashchain' -A 5
rg --type rust '/get-current-commitment' -A 5
rg --type rust '/swagger-ui' -A 5
rg --type rust '/api-docs/openapi.json' -A 5

Length of output: 3673


3-3: LGTM! But verify the usage of the new imports and annotations.

The changes to the imports and struct definitions are approved.

However, ensure that all references to the new imports and annotations are correctly used in the codebase.

Also applies to: 6-6, 8-13, 16-17, 20-22

Verification successful

Let's gather more context around the usage of the axum, utoipa, and ToSchema references to ensure they are correctly used.


All references to the new imports and annotations are correctly used.

The axum, utoipa, and ToSchema imports are consistently and correctly implemented in the src/webserver.rs file.

  • The axum crate is used for setting up the web server and routing.
  • The utoipa crate is used for API documentation.
  • The ToSchema annotation is applied to various structs for schema generation.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to the new imports and annotations are correctly used.

# Test: Search for references to the `axum` crate. Expect: Correct usage.
rg --type rust 'axum'

# Test: Search for references to the `utoipa` crate. Expect: Correct usage.
rg --type rust 'utoipa'

# Test: Search for references to the `ToSchema` annotation. Expect: Correct usage.
rg --type rust 'ToSchema'

Length of output: 780


Script:

#!/bin/bash
# Description: Gather more context around the usage of the new imports and annotations.

# Get more context around the usage of `axum`
rg --type rust 'axum' -A 10 -B 10

# Get more context around the usage of `utoipa`
rg --type rust 'utoipa' -A 10 -B 10

# Get more context around the usage of `ToSchema`
rg --type rust 'ToSchema' -A 10 -B 10

Length of output: 8397


115-141: LGTM! But verify the usage of the update_entry function.

The changes to the update_entry function are approved.

However, ensure that all references to the update_entry function are correctly used in the codebase.

Verification successful

The update_entry function in src/webserver.rs is correctly used.

The references to the update_entry function in src/webserver.rs are consistent and correctly implemented. The other instances of update_entry in src/node_types/sequencer.rs are unrelated to the update_entry function in src/webserver.rs.

  • src/webserver.rs: Correct usage in the webserver routes.
  • src/node_types/sequencer.rs: Instances are method calls or variable names, not function definitions.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to the `update_entry` function are correctly used.

# Test: Search for references to the `update_entry` function. Expect: Correct usage.
rg --type rust 'update_entry'

Length of output: 1446


Script:

#!/bin/bash
# Description: List functions in src/node_types/sequencer.rs to identify if there is another `update_entry` function.

# List all function definitions in src/node_types/sequencer.rs
ast-grep --lang rust --pattern 'fn $_($_) -> $_ { $$$ }' src/node_types/sequencer.rs

Length of output: 86


Script:

#!/bin/bash
# Description: List functions in src/node_types/sequencer.rs to identify if there is another `update_entry` function.

# List all function definitions in src/node_types/sequencer.rs
ast-grep --lang rust --pattern 'fn $_($_) { $$$ }' src/node_types/sequencer.rs

Length of output: 293


76-84: LGTM! But verify the usage of the SignedContent trait for UpdateEntryJson.

The changes to the UpdateEntryJson struct are approved.

However, ensure that all references to the SignedContent trait for UpdateEntryJson are correctly used in the codebase.

Verification successful

LGTM! But verify the usage of the SignedContent trait for UpdateEntryJson.

The changes to the UpdateEntryJson struct are approved.

  • The SignedContent trait implementation for UpdateEntryJson is correctly utilized in the codebase, particularly in the validate_and_queue_update function in src/node_types/sequencer.rs.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to the `SignedContent` trait for `UpdateEntryJson` are correctly used.

# Test: Search for references to the `SignedContent` trait for `UpdateEntryJson`. Expect: Correct usage.
rg --type rust 'SignedContent.*UpdateEntryJson'

Length of output: 107


Script:

#!/bin/bash
# Description: Broaden the search to look for `UpdateEntryJson` usage in the codebase to ensure it is used correctly with the `SignedContent` trait.

# Test: Search for references to `UpdateEntryJson` in the codebase.
rg --type rust 'UpdateEntryJson'

Length of output: 669


Script:

#!/bin/bash
# Description: Extract and review the context around the found references for `UpdateEntryJson` in `src/node_types/sequencer.rs`.

# Test: Show context lines around references to `UpdateEntryJson` in `src/node_types/sequencer.rs`.
rg --type rust --context 5 'UpdateEntryJson' src/node_types/sequencer.rs

Length of output: 1617


174-185: LGTM! But verify the usage of the get_commitment function.

The changes to the get_commitment function are approved.

However, ensure that all references to the get_commitment function are correctly used in the codebase.

Verification successful

LGTM! But verify the usage of the get_commitment function.

The changes to the get_commitment function are approved, and all references to the get_commitment function are correctly used in the codebase.

  • src/webserver.rs: Correctly used in the webserver context.
  • tests/integration_tests.rs: Used in integration tests to retrieve commitments.
  • src/utils.rs: Used in utility functions to validate epochs.
  • src/zk_snark.rs: Used in zk-snark operations to serialize and deserialize proofs.
  • src/node_types/sequencer.rs: Used in sequencer methods to manage commitments.
  • src/storage.rs: Used in storage interfaces to get commitments for specific epochs.
  • src/da/mock.rs: Used in data availability mocks to simulate commitment retrieval.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to the `get_commitment` function are correctly used.

# Test: Search for references to the `get_commitment` function. Expect: Correct usage.
rg --type rust 'get_commitment'

Length of output: 2084


Script:

#!/bin/bash
# Extract lines around references to `get_commitment` to verify usage.

# Get context lines around matches in relevant files
rg --type rust 'get_commitment' -A 3 -B 3 tests/integration_tests.rs src/utils.rs src/zk_snark.rs src/node_types/sequencer.rs src/storage.rs src/da/mock.rs

Length of output: 8386


62-74: LGTM! But verify the usage of the utoipa annotations.

The changes to the API documentation with utoipa annotations are approved.

However, ensure that all references to the utoipa annotations are correctly used in the codebase.

Also applies to: 115-126, 145-157, 174-181

Verification successful

The utoipa annotations are correctly applied to the relevant functions.

The utoipa::path annotations for the update_entry, get_hashchain, and get_commitment functions are correctly implemented with appropriate paths, request bodies, and responses.

  • update_entry: Correctly annotated with POST method, path /update-entry, and appropriate responses.
  • get_hashchain: Correctly annotated with POST method, path /get-hashchain, and appropriate responses.
  • get_commitment: Correctly annotated with GET method, path /get-current-commitment, and appropriate responses.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to the `utoipa` annotations are correctly used.

# Test: Search for references to the `utoipa` annotations. Expect: Correct usage.
rg --type rust 'utoipa'

Length of output: 223


Script:

#!/bin/bash
# Description: Verify the correctness of `utoipa::path` annotations.

# Search for `utoipa::path` annotations and retrieve the related function definitions.
ast-grep --lang rust --pattern $'#[utoipa::path($_)] fn $_($$$_) { $$$ }'

Length of output: 536


Script:

#!/bin/bash
# Description: Verify the correctness of `utoipa::path` annotations.

# Search for `utoipa::path` annotations and retrieve the related function definitions.
ast-grep --lang rust --pattern $'#[utoipa::path($_)]' -A 5

Length of output: 59


Script:

#!/bin/bash
# Description: Verify the correctness of `utoipa::path` annotations.

# Search for `utoipa::path` annotations and include additional context lines to capture related function definitions.
rg --type rust '#\[utoipa::path' -A 10

Length of output: 1736

src/utils.rs (3)

45-45: LGTM!

The error message update is correct and improves clarity.


113-115: LGTM!

The renaming of the trait and method improves clarity and aligns with the changes in the verify_signature function.


Line range hint 120-137:
LGTM!

The function logic is correct and aligns with the changes in the SignedContent trait.

src/node_types/sequencer.rs (6)

42-44: LGTM!

The new fields enhance the functionality related to data processing and commitment management.


200-203: LGTM!

The method logic is correct and improves the functionality of the Sequencer.


226-239: LGTM!

The method logic is correct and streamlines the epoch finalization process.


271-279: LGTM!

The method logic is correct and improves the handling of updates.


Line range hint 283-427:
LGTM!

The method logic is correct and improves the efficiency of entry updates.


436-462: LGTM!

The method logic is correct and enhances the operational capabilities of the Sequencer.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT
Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

* feat: operation queue

* refactor and beginning test

* progress

* balls deep in redis debugging

* no more deadlock but proof verification fails

* test passes if we don't derive tree, but is tree state fucked then?

* yanking out derive_tree

* renaming Signable trait and method names
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (1)
src/utils.rs (1)

23-25: Consider renaming to improve readability.

The TODO comment suggests renaming is_revoked to is_not_revoked for better readability.

-/// TODO(@distractedm1nd): is_revoked > is_not_revoked, for readability

@distractedm1nd distractedm1nd merged commit 573c9e2 into main Jul 20, 2024
1 check failed
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.

1 participant