Skip to content

Conversation

@dev-jodee
Copy link
Contributor

@dev-jodee dev-jodee commented Nov 24, 2025

Work done by @lgalabru

Commits of #247 but rebased on main / newer version

(From https://github.com/txtx/kora/tree/fix/limit-singletons-access)


Important

Refactor code to pass config as a parameter to functions, improving modularity and reducing global state dependency.

  • Behavior:
    • Refactor to pass config as a parameter to functions instead of accessing it globally.
    • Affects functions like validate_transaction, check_transaction_usage_limit, and validate_with_result.
  • Files:
    • transaction/versioned_transaction.rs: Updates VersionedTransactionResolved methods to accept config.
    • usage_limit/usage_tracker.rs: Modifies check_transaction_usage_limit to require config.
    • validator/config_validator.rs: Changes validate_with_result to use config parameter.
  • Misc:
    • Improves modularity and testability by reducing global state dependency.
    • Updates tests to accommodate the new function signatures.

This description was created by Ellipsis for b6d780f. You can customize this summary. It will automatically update as commits are pushed.

📊 Unit Test Coverage

Coverage

Unit Test Coverage: 81.0%

View Detailed Coverage Report

Work done by @lgalabru

Commits of #247 but rebased on main / newer version
@dev-jodee dev-jodee requested a review from amilz November 24, 2025 22:12
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to b6d780f in 2 minutes and 3 seconds. Click for details.
  • Reviewed 3325 lines of code in 16 files
  • Skipped 0 files when reviewing.
  • Skipped posting 7 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. crates/lib/src/validator/transaction_validator.rs:25
  • Draft comment:
    The field _price_source is stored and initialized (line 25 and line 51) but never used in any validation. Consider removing it if it isn’t needed.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. crates/lib/src/validator/transaction_validator.rs:168
  • Draft comment:
    The validation of system and SPL instructions relies on custom macros (e.g. validate_system! and validate_spl!). Ensure these macros are well‐documented and handle all edge cases for clarity and maintainability.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. crates/lib/src/validator/transaction_validator.rs:80
  • Draft comment:
    In fetch_and_validate_token_mint, the error returned when the mint isn’t in the allowed_tokens list could include additional context (e.g. what tokens are allowed) to help debugging.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50% None
4. crates/lib/src/validator/transaction_validator.rs:369
  • Draft comment:
    The strict pricing validation (validate_strict_pricing_with_fee) is implemented clearly. Consider adding more inline comments explaining the rationale behind the fixed price versus total fee calculation for future maintainers.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment suggests adding inline comments to explain the rationale behind a calculation. This is a request for more documentation, which is not allowed according to the rules. The comment does not provide a specific code suggestion or identify a potential issue with the code itself.
5. crates/lib/src/validator/transaction_validator.rs:101
  • Draft comment:
    The validate_transaction method begins by checking for empty instructions and account keys. This is a good practice; ensure that this early validation is comprehensive, especially when inner instructions might be parsed later.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. crates/lib/src/validator/transaction_validator.rs:142
  • Draft comment:
    The signature validation simply checks length and emptiness. Ensure that this is sufficient and that any cryptographic signature verification is handled elsewhere.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50% None
7. crates/lib/src/token/token.rs:786
  • Draft comment:
    Typographical error: The numeric literal is written as 1,000,000 with commas. In Rust, integer literals should use underscores (e.g. 1_000_000) or no separators, as commas will result in a compilation error. Please fix this.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_IPnE1v8tzjwDcgxd

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@amilz amilz assigned amilz and dev-jodee and unassigned amilz Nov 24, 2025
Copy link
Contributor

@amilz amilz left a comment

Choose a reason for hiding this comment

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

lgtm -- per our chat let's just make sure to have a proper release/branch plan before merging

@dev-jodee dev-jodee merged commit 68145ec into main Nov 25, 2025
9 checks passed
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.

3 participants