Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(evm): Update SDK to Support New ERC20Handler with Optional Contract Call #504

Merged
merged 16 commits into from
Sep 10, 2024

Conversation

saadahmsiddiqui
Copy link
Member

@saadahmsiddiqui saadahmsiddiqui commented Aug 28, 2024

Implementation details

With the introduction of the new ERC20Handler, the SDK needs to be updated to handle the enhanced functionality that supports both ERC20 token transfers and optional contract calls in a single transaction. New ERC20Handler PR

  • Modify the SDK to work with the updated ERC20Handler by allowing it to handle additional data that encodes contract calls alongside ERC20 transfers.
  • Maintain compatibility with existing handlers and workflows that do not involve contract calls, ensuring that the SDK remains functional for all users.

Closes: #493

Testing details

  • Unit Tests: Develop unit tests that cover scenarios where both ERC20 transfers and contract calls are involved.
  • Compatibility Tests: Ensure that existing functionality remains unaffected by the changes.
  • Error Handling: Test how the SDK handles invalid or failed contract calls within the transaction flow.

Acceptance Criteria

  • The SDK supports interactions with the new ERC20Handler, allowing for both ERC20 transfers and optional contract calls within the same transaction.
  • The SDK maintains backward compatibility and continues to function as expected with existing handlers.
  • All new functionality is covered by tests, ensuring reliable and consistent behavior.

@saadahmsiddiqui saadahmsiddiqui changed the title feat: Update SDK to Support New ERC20Handler with Optional Contract Call feat(evm): Update SDK to Support New ERC20Handler with Optional Contract Call Aug 28, 2024
@saadahmsiddiqui saadahmsiddiqui self-assigned this Aug 28, 2024
@lastperson
Copy link

Overall correct, though on the contracts side optionalGas if included must be accompanied by the optionalMessage, otherwise contract call would not happen. So allowing users to only include gas might create a false impression that it affects anything.

Lykhoyda
Lykhoyda previously approved these changes Sep 9, 2024
wainola
wainola previously approved these changes Sep 9, 2024
Copy link
Contributor

@wainola wainola left a comment

Choose a reason for hiding this comment

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

Left two comments

examples/evm-to-evm-fungible-transfer/src/transfer.ts Outdated Show resolved Hide resolved
packages/evm/src/utils/depositFn.ts Outdated Show resolved Hide resolved
@saadahmsiddiqui saadahmsiddiqui dismissed stale reviews from wainola and Lykhoyda via 4ec8bb7 September 9, 2024 12:12
@saadahmsiddiqui saadahmsiddiqui merged commit 2cb56a2 into main Sep 10, 2024
3 checks passed
@saadahmsiddiqui saadahmsiddiqui deleted the feat/erc20-generic-handler branch September 10, 2024 08:57
saadahmsiddiqui pushed a commit that referenced this pull request Sep 10, 2024
🤖 I have created a release *beep* *boop*
---


##
[1.3.0](evm-v1.2.0...evm-v1.3.0)
(2024-09-10)


### Features

* **evm:** Update SDK to Support New ERC20Handler with Optional Contract
Call ([#504](#504))
([2cb56a2](2cb56a2))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
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.

Update SDK to Support New ERC20Handler with Optional Contract Call
4 participants