Skip to content

Conversation

@0xFirekeeper
Copy link
Member

@0xFirekeeper 0xFirekeeper commented Nov 7, 2024

An alternative to ERC20_Transfer that might make more sense to some people.


PR-Codex overview

This PR introduces a new test for transferring ERC20 tokens and modifies the Transfer method to handle both native tokens and ERC20 tokens based on an optional tokenAddress parameter.

Detailed summary

  • Added a new test method TransferERC20Override in Thirdweb.Extensions.Tests.cs.
  • Updated XML documentation for the Transfer method to describe ERC20 token transfer.
  • Modified the Transfer method to accept an optional tokenAddress parameter.
  • Implemented logic to transfer ERC20 tokens if tokenAddress is provided.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

An alternative to ERC20_Transfer that might make more sense to some people.
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@d4be241). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #96   +/-   ##
=======================================
  Coverage        ?   78.24%           
=======================================
  Files           ?       28           
  Lines           ?     4363           
  Branches        ?      611           
=======================================
  Hits            ?     3414           
  Misses          ?      786           
  Partials        ?      163           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@0xFirekeeper 0xFirekeeper merged commit 733de6a into main Nov 7, 2024
1 check passed
@0xFirekeeper 0xFirekeeper deleted the firekeeper/wallet-transfer-erc20-override branch November 7, 2024 15:09
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