Skip to content

Conversation

@dev-jodee
Copy link
Contributor

@dev-jodee dev-jodee commented Sep 23, 2025

… and other scenarios

  • Introduced new tests for fee payer policy violations, including SOL and SPL transfer restrictions.
  • Added adversarial test suite to validate fee payer behavior under restrictive policies.
  • Updated configuration files for testing scenarios and added new test cases for various fee payer policy violations.
  • Refactored existing test utilities to support new test cases and improved error assertions.

Important

Add adversarial tests for fee payer policy violations and update configurations for enhanced transaction validation.

  • Tests:
    • Added adversarial tests for fee payer policy violations in fee_payer_policy_violations.rs, including SOL and SPL transfer restrictions.
    • Introduced adversarial test suite in adversarial/main.rs for security and robustness testing, covering program validation attacks and invalid token states.
    • Added tests for fee payer exploitation in fee_payer_exploitation.rs.
  • Configuration:
    • Updated tests/Cargo.toml to include new test suites adversarial and fee_payer_policy.
    • Added fee-payer-policy-test.toml for restrictive policy configurations.
  • Validation:
    • Enhanced TransactionValidator in transaction_validator.rs to include detailed error messages for fee payer policy violations.
    • Refactored check_if_allowed function to include instruction_type for better error reporting.
  • Misc:
    • Renamed multi-signers.toml to signers-multi.toml in CLAUDE.md and test_cases.toml.

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

📊 Unit Test Coverage

Coverage

Unit Test Coverage: 85.8%

View Detailed Coverage Report

… and other scenarios

- Introduced new tests for fee payer policy violations, including SOL and SPL transfer restrictions.
- Added adversarial test suite to validate fee payer behavior under restrictive policies.
- Updated configuration files for testing scenarios and added new test cases for various fee payer policy violations.
- Refactored existing test utilities to support new test cases and improved error assertions.
@dev-jodee dev-jodee requested a review from amilz September 23, 2025 16:31
@linear
Copy link

linear bot commented Sep 23, 2025

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 3813005 in 2 minutes and 40 seconds. Click for details.
  • Reviewed 966 lines of code in 16 files
  • Skipped 0 files when reviewing.
  • Skipped posting 14 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. CLAUDE.md:113
  • Draft comment:
    Updated config path for multi-signer tests now points to 'tests/src/common/fixtures/signers-multi.toml'. Verify consistency with file rename.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the author to verify consistency with a file rename, which is against the rules as it asks for confirmation. It doesn't provide a specific suggestion or point out a specific issue with the code.
2. tests/Cargo.toml:11
  • Draft comment:
    New [[test]] entries for 'adversarial' and 'fee_payer_policy' are added. Confirm that these names are used uniformly across the test runner configuration.
  • 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. tests/adversarial/fee_payer_exploitation.rs:35
  • Draft comment:
    Test case for SOL transfer exploitation properly simulates insufficient token payment. Ensure that the expected error message 'Insufficient token payment' remains stable.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the author to ensure that the expected error message remains stable, which is a form of asking for confirmation or verification. This violates the rule against asking the PR author to ensure behavior is intended or tested. Therefore, this comment should be removed.
4. tests/adversarial/program_validation.rs:27
  • Draft comment:
    Disallowed memo program test correctly expects an error with the message indicating the program is not allowed. No issues found.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
5. tests/adversarial/token_states.rs:91
  • Draft comment:
    Frozen token account test asserts error 'custom program error: 0x11'. Verify that this error code remains unchanged in the underlying program.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to verify that an error code remains unchanged, which is against the rules of asking for confirmation or verification. It doesn't provide a specific suggestion or point out a specific issue with the code.
6. tests/fee_payer_policy/fee_payer_policy_violations.rs:33
  • Draft comment:
    Fee payer policy violation tests for SOL transfers and other actions are comprehensive. Double-check that the expected error message 'Fee payer cannot be used as source account' exactly matches the RPC response.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to double-check the error message, which violates the rule against asking for confirmation or verification. It does not provide a specific suggestion or point out a clear issue with the code.
7. tests/src/common/assertions.rs:124
  • Draft comment:
    RpcErrorAssertions trait implementations for both RpcError and AnyhowError are useful. Consider adding inline documentation for each method for clarity.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
8. tests/src/common/auth_helpers.rs:40
  • Draft comment:
    Auth helper functions correctly build requests with custom headers. Ensure that test bodies (e.g., JSON_TEST_BODY) stay in sync with the expected payload for authentication tests.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
9. tests/src/common/fixtures/fee-payer-policy-test.toml:51
  • Draft comment:
    Fee payer policy configuration file sets all policies to false. Confirm that the allowed and disallowed program lists match production rules.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
10. tests/src/common/setup.rs:391
  • Draft comment:
    Updated mint_tokens_to_account and mint_tokens_2022_to_account functions are now public. Ensure they are used elsewhere consistently in tests.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the author to ensure that the updated functions are used consistently in tests. This falls under the rule of not asking the author to ensure that their change is tested, which is not allowed.
11. tests/src/common/transaction.rs:151
  • Draft comment:
    Added with_spl_payment_with_accounts method facilitates specifying token accounts explicitly. This is useful for fee payer tests—review that account ordering conforms to spl_token::instruction::transfer.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
12. tests/src/test_runner/output.rs:21
  • Draft comment:
    New 'FeePayerPolicy' variant added to TestPhaseColor; the chosen ANSI code (\x1b[39m]) should be double‑checked for visual consistency with other phases.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to double-check the ANSI code for visual consistency. This falls under the rule of not asking the author to double-check things. Therefore, this comment should be removed.
13. tests/src/test_runner/test_cases.toml:23
  • Draft comment:
    New test case for fee_payer_policy has been added and multi_signer signers file updated to 'signers-multi.toml'. Confirm that port assignments (e.g., 8086 for fee_payer_policy) do not conflict with other services.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to confirm that port assignments do not conflict with other services. This falls under asking the author to double-check things, which is against the rules. Therefore, this comment should be removed.
14. tests/fee_payer_policy/fee_payer_policy_violations.rs:191
  • Draft comment:
    Typo in comment: "Create a new token account to now affect other tests". Likely meant to say "to not affect other tests".
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% While this is technically a typo that changes the meaning of the comment (now vs not), fixing comment typos is a very minor issue. The meaning is fairly clear from context, and the comment is only documenting the test setup. This isn't affecting code behavior or making the code harder to understand. The typo does technically reverse the meaning of the comment. Someone might be confused about whether the test is supposed to affect other tests or not. While true, this is a test file and the actual behavior is determined by the code, not the comment. The code clearly shows creating a new account, so the intent is obvious regardless of the comment typo. This comment is too minor and not important enough to keep. Comment typos that don't significantly impact code understanding should not be called out in reviews.

Workflow ID: wflow_QCOy5YHoE49nOGpn

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

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. This line is still not showing on next line for some reason fyi.

{test stuff} 0.89s=== Completed Payment Address Tests ===

Copy link
Contributor

Choose a reason for hiding this comment

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

we explicitly have rules for transfers / token22 transfers. I'm wondering if we should test approve/burn with t22. i would expect the rules to govern all token programs.

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 e9304b9 in 1 minute and 8 seconds. Click for details.
  • Reviewed 152 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 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:178
  • Draft comment:
    Nice enhancement: the closure now distinguishes instruction types to provide more descriptive errors. Ensure the use of named interpolation (e.g. '{instruction_type}') is supported by your Rust edition, or consider explicitly passing the variable.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. tests/fee_payer_policy/fee_payer_policy_violations.rs:35
  • Draft comment:
    Updated error messages in tests now match the new descriptive errors. Verify that all tests consistently expect messages like "Fee payer cannot be used for 'System Transfer'".
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_QKOz1r11V2SquWvm

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

@dev-jodee dev-jodee merged commit cbf7225 into release/feature-freeze-for-audit Sep 23, 2025
8 of 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