feat: Changed transfer_transaction endpoint to actually return paymen…

[dev-jodee]

…t instruction, unsigned

• Modified the transfer_transaction function to automatically set the payment destination to Kora's address based on the selected signer, eliminating the need for a destination parameter in the request.
• Updated TypeScript SDK documentation and types to reflect the changes, ensuring clarity on the transaction's unsigned nature and the automatic destination handling.
• Adjusted tests to validate the new behavior and ensure proper functionality without the destination parameter.

closes #280

---

Important

transfer_transaction endpoint now auto-sets payment destination to Kora's address, deprecating the endpoint in favor of getPaymentInstruction.

• Behavior:
  • transfer_transaction function now automatically sets payment destination to Kora's address based on signer, removing destination parameter.
  • Endpoint deprecated in favor of getPaymentInstruction.
• SDK:
  • Updated TypeScript SDK documentation and types to reflect changes.
  • transferTransaction method in client.ts marked as deprecated.
• Tests:
  • Adjusted tests in integration.test.ts and unit.test.ts to validate new behavior without destination parameter.
  • Deprecated tests for transferTransaction in transfers.rs and token_2022_test.rs.

^{This description was created by for 1f40ccc. 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

[github-actions]

📊 TypeScript Coverage Report

Coverage: 82.0%

View detailed report

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

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to b775bf0 in 2 minutes and 29 seconds. Click for details.

• Reviewed 778 lines of code in 8 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/rpc_server/method/transfer_transaction.rs:72

• Draft comment:
  Since the destination parameter is removed, ensure that the validation and error messaging (e.g. for disallowed accounts) clearly reflect that the destination is auto-filled.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

2. sdks/ts/src/client.ts:250

• Draft comment:
  The JSDoc and implementation for transferTransaction now include payment_destination and signer_pubkey – ensure that the decoding of instructions (via getInstructionsFromBase64Message) gracefully handles empty or legacy messages.
• 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 decoding of instructions handles empty or legacy messages gracefully. This is a request for confirmation of behavior, which violates the rule against asking the author to ensure behavior is intended. Therefore, this comment should be removed.

3. sdks/ts/src/types/index.ts:86

• Draft comment:
  The updated TransferTransactionResponse interface now correctly documents the new 'payment_destination' field along with the unsigned transaction details. This looks good.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

4. sdks/ts/test/unit.test.ts:299

• Draft comment:
  Unit tests updated to remove the destination parameter and include the new 'payment_destination' field – ensure that all mock responses match the new API spec.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

5. tests/multi_signer/signer_management.rs:97

• Draft comment:
  Tests now rely on the signer_key parameter to auto-fill the destination. Confirm that using the first signer’s public key in both estimate and transfer calls achieves the intended consistency.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

6. tests/rpc/transfers.rs:15

• Draft comment:
  Tests correctly remove the destination param, relying on Kora to auto-fill payment_destination. Ensure that unsigned transaction decoding remains robust.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

7. tests/tokens/token_2022_test.rs:12

• Draft comment:
  Token 2022 tests are updated to validate both the presence of 'payment_destination' and decodability of the unsigned transaction. These changes are consistent; ensure to monitor for any edge cases in ATA creation.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_ChgqZe2oOeEnLx5b

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

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 965f46d in 1 minute and 11 seconds. Click for details.

• Reviewed 789 lines of code in 10 files
• Skipped 0 files when reviewing.
• Skipped posting 8 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. tests/multi_signer/signer_management.rs:190

• Draft comment:
  In the invalid signer key test, consider asserting on the error message content for clearer diagnosis.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

2. tests/multi_signer/signer_management.rs:152

• Draft comment:
  The round-robin test asserts at least 2 signers are used. Ensure the test context always provides multiple fee-payers or note adjustments if config changes.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

3. tests/rpc/transfers.rs:6

• Draft comment:
  Verify that the unsigned transaction response now includes the 'signer_pubkey' field and omits parsed instructions per the new spec.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

4. tests/tokens/token_2022_test.rs:12

• Draft comment:
  Test descriptions now include '(DEPRECATED endpoint)'; ensure these labels are maintained until the endpoint is removed.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

5. tests/tokens/token_2022_test.rs:80

• Draft comment:
  Consider refactoring repetitive transaction building steps (for legacy and v0 variants) to reduce duplication in tests.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

6. tests/tokens/token_2022_test.rs:120

• Draft comment:
  The simulation checks (assert simulated_tx.value.err is none) are effective; consider adding validation of key instructions if needed.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

7. tests/tokens/token_2022_test.rs:185

• Draft comment:
  For lookup table tests, ensure the fixture for the lookup table is kept up-to-date with actual deployment parameters.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

8. tests/tokens/token_2022_test.rs:360

• Draft comment:
  The sign-transaction-if-paid tests are comprehensive; consider documenting expected fee payer pool behavior if changes occur.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_nKFyDno5Wrpg0AfH

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

[amilz]

Per chat, let's keep SDK response type as-is. Then LGTM

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 6dbf6b3 in 50 seconds. Click for details.

• Reviewed 43 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. sdks/ts/src/client.ts:280

• Draft comment:
  Parsed instructions are now added to the response via getInstructionsFromBase64Message. Ensure that this utility handles empty or invalid messages gracefully.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

2. sdks/ts/src/types/index.ts:97

• Draft comment:
  The TransferTransactionResponse interface now includes an 'instructions' field. Verify that its structure matches the parsed output from getInstructionsFromBase64Message.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_YXJZVL8TNHvD91Kx

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

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 1f40ccc in 57 seconds. Click for details.

• Reviewed 12 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 1 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/test/unit.test.ts:307

• Draft comment:
  Added the explicit 'instructions: []' property in the expected transfer transaction response. Make sure this aligns with the intended behavior—if the endpoint should now return a populated payment instruction rather than an empty array, update the test to validate its structure.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 30% <= threshold 50% The comment is asking the PR author to ensure that the behavior is intended and to update the test if necessary. This violates the rule against asking the author to confirm their intention or ensure behavior. However, it does suggest updating the test, which is specific. Overall, the comment leans more towards asking for confirmation rather than providing a specific suggestion or request for a test.

Workflow ID: wflow_zkO01zshK3rG9Y2s

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