Skip to content

chore: Add sig_verify option to transaction request types#211

Merged
dev-jodee merged 7 commits intomainfrom
chore/ts-sdk-update-sig-verify
Aug 29, 2025
Merged

chore: Add sig_verify option to transaction request types#211
dev-jodee merged 7 commits intomainfrom
chore/ts-sdk-update-sig-verify

Conversation

@amilz
Copy link
Copy Markdown
Contributor

@amilz amilz commented Aug 29, 2025

Adds the optional sig_verify boolean to transaction-related request interfaces and methods in TS SDK. Updated Documentation to reflect this new parameter.


Important

Adds optional sig_verify parameter to transaction requests in TypeScript SDK for optional signer verification during simulation.

  • Behavior:
    • Adds optional sig_verify boolean to transaction-related request interfaces in types/index.ts.
    • Updates methods in client.ts to handle sig_verify for transaction simulation.
    • Documentation in README.md updated to include sig_verify parameter.
  • Refactor:
    • Changes hex_to_bytes() in utils.rs to use is_multiple_of(2) for even length check.

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

📊 Test Coverage

Coverage

Coverage: 86.3%

View Detailed Coverage Report

dev-jodee and others added 6 commits August 29, 2025 12:28
- Introduced `sig_verify` flag to `EstimateTransactionFeeRequest`, `SignAndSendTransactionRequest`, `SignTransactionIfPaidRequest`, and `SignTransactionRequest` for controlling signature verification during transaction simulation.
- Added `default_sig_verify` function to provide a default value for the `sig_verify` flag.
- Updated transaction resolution methods to utilize the `sig_verify` flag when simulating transactions.
- Enhanced tests to cover scenarios with the new `sig_verify` functionality.
Introduces the optional sig_verify boolean to transaction-related request interfaces and methods, allowing for optional signer verification during transaction simulation. Documentation and client method signatures are updated to reflect this new parameter.
@amilz amilz requested a review from dev-jodee August 29, 2025 18:47
@amilz amilz self-assigned this Aug 29, 2025
Copy link
Copy Markdown
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 f0e4526 in 50 seconds. Click for details.
  • Reviewed 142 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 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. sdks/ts/docs/README.md:432
  • Draft comment:
    New 'sig_verify' parameter added across endpoints. Consider standardizing its ordering with 'signer_key' and ensuring anchor IDs (e.g. sig_verify-1, sig_verify-2, etc.) are consistent for clarity.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50% None
2. sdks/ts/src/client.ts:332
  • Draft comment:
    Optional 'sig_verify' is correctly propagated in getPaymentInstruction. Consider defaulting it explicitly to false during destructuring for clarity if not provided.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
3. sdks/ts/src/types/index.ts:27
  • Draft comment:
    New optional 'sig_verify' flag has been added to several request interfaces. Consider aligning the property order with documentation (e.g. docs list 'sig_verify' before 'signer_key' while interfaces list it after) for consistency.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50% None

Workflow ID: wflow_2V9uuSOLpCsMDllc

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

@dev-jodee dev-jodee changed the base branch from feat/pro-261-allow-sig-verify-false-on-txn-simulation to main August 29, 2025 18:48
@github-actions
Copy link
Copy Markdown

📊 TypeScript Coverage Report

Coverage: 82.0%

View detailed report

Coverage artifacts have been uploaded to this workflow run.
View Artifacts

Copy link
Copy Markdown
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 8799551 in 1 minute and 33 seconds. Click for details.
  • Reviewed 444 lines of code in 11 files
  • Skipped 1 files when reviewing.
  • Skipped posting 12 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. .github/badges/coverage.json:1
  • Draft comment:
    Coverage dropped from 86.3% to 86.2%. Confirm that this slight drop is expected.
  • 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 if the drop in coverage is expected. This falls under the rule of not asking the author to confirm their intention or to ensure behavior is intended. Therefore, this comment should be removed.
2. README.md:1
  • Draft comment:
    Updated README with centered logo and reformatted layout; verify all link paths (e.g. docs/README.md) work as intended.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is asking the PR author to verify link paths, which falls under the rule of not asking the author to double-check things. It doesn't provide a specific suggestion or point out a specific issue.
3. crates/lib/src/metrics/README.md:89
  • Draft comment:
    Relative path for the Kora Monitoring Guide has been updated. Please double-check that '../../../../docs/operators/MONITORING.md' is the correct new path.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment asks the PR author to double-check a file path, which violates the rule against asking the author to confirm or double-check things. It does not provide a specific suggestion or point out a clear issue with the code.
4. crates/lib/src/rpc_server/method/estimate_transaction_fee.rs:70
  • Draft comment:
    The fee calculation now applies a price model via get_required_lamports. Verify that the computed fee including the margin is as expected.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is asking the PR author to verify that the computed fee is as expected, which falls under the rule of not asking the author to ensure the behavior is intended or to double-check things. Therefore, this comment should be removed.
5. crates/lib/src/signer/utils.rs:6
  • Draft comment:
    Using is_multiple_of(2) improves clarity over modulus check for even-length validation.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
6. crates/lib/src/transaction/versioned_transaction.rs:150
  • Draft comment:
    Explicitly setting the commitment in the simulation config ensures the RPC client’s settings are honored. This looks like a positive improvement.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
7. crates/lib/src/validator/transaction_validator.rs:1
  • Draft comment:
    Removal of the unused 'ValidationMode' enum tidies up the code. Ensure that any client code depended on this is updated accordingly.
  • Reason this comment was not posted:
    Confidence changes required: 20% <= threshold 50% None
8. docs/getting-started/demo/server/kora.toml:1
  • Draft comment:
    New caching and token2022 configuration sections have been added. Verify that these settings align with the intended configuration and documentation.
  • 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.
9. docs/operators/deploy/sample/kora.toml:1
  • Draft comment:
    The sample deploy configuration now includes caching, enhanced validation, and metrics settings. Please confirm that all token addresses and paths are current.
  • 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.
10. kora.toml:1
  • Draft comment:
    Main kora.toml has updated caching and validation.token2022 sections. Ensure that comments accurately describe usage and default behavior.
  • 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.
11. sdks/ts/README.md:37
  • Draft comment:
    The TypeScript SDK README now points to the external API Reference and Quick Start guides. Confirm that the updated CI test command ('pnpm test:ci:integration') is correct.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
12. docs/operators/deploy/sample/kora.toml:64
  • Draft comment:
    Typographical note: there's a double '#' in the comment ('# # 10% margin'). Likely this is an unintentional extra '#' and should be removed for clarity.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While the observation about the double '#' is technically correct, this is an extremely minor issue that doesn't affect functionality or even readability significantly. The comment is still perfectly readable and understandable. This kind of nitpick about comment formatting violates the rule about not making obvious or unimportant comments. The double '#' could potentially be confusing to future readers. Also, maintaining consistent comment style is important for code quality. While consistency is good, this is too minor of an issue to warrant a PR comment. The meaning is clear despite the typo, and this kind of minor formatting issue can be addressed during regular maintenance if needed. Delete this comment as it violates the rule about not making obvious or unimportant comments. The issue is too minor to warrant attention in a PR review.

Workflow ID: wflow_cEyGkJRTmthOjEf9

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 8458428 into main Aug 29, 2025
4 checks passed
@dev-jodee dev-jodee deleted the chore/ts-sdk-update-sig-verify branch January 6, 2026 19:16
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.

2 participants