Feat/improved warnings for cli configs

[dev-jodee]

Important

Introduces strict pricing for fixed price models, validates risky Token2022 extensions, and enhances fee payer policy warnings.

• Behavior:
  • Adds strict flag to PriceModel::Fixed in price.rs to enforce strict pricing rules.
  • Validates strict pricing in TransactionValidator::validate_strict_pricing_with_fee() in transaction_validator.rs.
  • Adds warnings for risky Token2022 extensions in ConfigValidator::check_token_mint_extensions() in config_validator.rs.
  • Enhances fee payer policy warnings in ConfigValidator::validate_fee_payer_policy() in config_validator.rs.
• Tests:
  • Adds tests for strict pricing in transaction_validator.rs.
  • Adds tests for Token2022 extension warnings in config_validator.rs.
  • Adds tests for fee payer policy warnings in config_validator.rs.

^{This description was created by for cb2960f. You can customize this summary. It will automatically update as commits are pushed.}

📊 Unit Test Coverage

Unit Test Coverage: 80.8%

View Detailed Coverage Report

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to cb2960f in 7 minutes and 3 seconds. Click for details.

• Reviewed 1006 lines of code in 8 files
• Skipped 0 files when reviewing.
• Skipped posting 5 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:364

• Draft comment:
  In validate_strict_pricing_with_fee, the local variable names (fixed_price_lamports vs total_fee_lamports from get_total_fee_lamports) are a bit confusing. It may help to rename them (e.g. config_fixed_fee and computed_fee) so that it’s clearer that the fixed fee from config must cover the sum of sub‐fees.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% This is a code quality/clarity suggestion about variable naming. The comment is about code that was added in the diff (lines 364-390 are new). The suggestion is somewhat subjective - the current names aren't objectively wrong, but they could be clearer. The comment suggests specific alternative names. However, looking at the code, I need to understand what these variables actually represent. Without seeing the TotalFeeCalculation struct definition, it's hard to know if the current names are truly confusing or if the suggested names would be better. The comment is making an assumption about what would be clearer, but this is subjective. According to the rules, "Comments that suggest code quality refactors are good! But only if they are actionable and clear." This suggestion is actionable (specific rename) but whether it's truly an improvement is debatable without more context. The comment is subjective about what constitutes "confusing" naming. Without understanding the full context of TotalFeeCalculation and how these fields are used elsewhere in the codebase, it's hard to say definitively that the suggested names are better. The current names do describe what they contain (lamports values), and the suggested names might not be more clear without additional context. While the suggestion is subjective, variable naming clarity is important for maintainability. However, the comment doesn't provide strong evidence that the current names are actually problematic - it just says they "may help" and could be "clearer". This is a weak suggestion that falls into the category of "nice to have" rather than "clearly needed". This is a subjective code quality suggestion about variable naming without strong evidence that the current names are problematic. The comment uses hedging language ("a bit confusing", "may help") which suggests uncertainty. Given the rules to only keep comments with strong evidence of issues, this should be deleted.

2. crates/lib/src/validator/transaction_validator.rs:148

• Draft comment:
  The use of macros (validate_system!, validate_spl!, validate_spl_multisig!) for instruction validations is clever but their implementation isn’t visible here. Ensure that these macros are well documented so that future maintainers can understand the matching pattern and the risk/flag logic.
• 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:112

• Draft comment:
  In validate_transaction, a chain of validations (signatures, programs, transfer amounts, disallowed accounts, fee payer usage) is performed. Consider adding more detailed logging or comments here to indicate which step failed when an error occurs, to help with debugging complex transactions.
• Reason this comment was not posted:
  Confidence changes required: 30% <= threshold 50% None

4. crates/lib/src/validator/transaction_validator.rs:31

• Draft comment:
  The TransactionValidator::new() function calls get_config() and converts allowed programs, tokens, and disallowed accounts to Pubkey. This design is clear, but if get_config() is expensive it might be worth caching the parsed config inside the validator instance.
• Reason this comment was not posted:
  Confidence changes required: 40% <= threshold 50% None

5. crates/lib/src/validator/transaction_validator.rs:75

• Draft comment:
  In fetch_and_validate_token_mint, the method checks if the mint is in allowed_tokens then calls TokenUtil::get_mint. Consider logging a warning when a mint is rejected, to aid in debugging misconfigurations.
• Reason this comment was not posted:
  Confidence changes required: 30% <= threshold 50% None

Workflow ID: wflow_Xo5ujmERifqVsu9V

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