-
Notifications
You must be signed in to change notification settings - Fork 11
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
Support sending new native USDC #484
Conversation
Move commented ABI to new file that is not part of the deployment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice and clean.
|
||
NATIVE_USDC_TRANSFER_CONTRACT_ABI: [ | ||
'function transfer(address token, uint256 amount, address target, uint256 fee)', | ||
'function transferWithPermit(address token, uint256 amount, address target, uint256 fee, uint256 value, bytes32 sigR, bytes32 sigS, uint8 sigV)', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is value
the permit?
Can this name be changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is the permit amount yes. Unfortunately, it cannot be changed, as the hash of the function signature is used for it's identification, meaning changing any argument name changes the ID.
const domain = { | ||
name: 'USD Coin', // This is currently the same for testnet and mainnet | ||
version: '2', // This is currently the same for testnet and mainnet | ||
verifyingContract: CONFIG.NATIVE_USDC_CONTRACT_ADDRESS, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder whether the old contract should actually also become LEGACY_USDC_CONTRACT_ADDRESS
or BRIDGED_USDC_CONTRACT_ADDRESS
, now that native usdc is meant to become the main usdc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be done, yes. I will do it in a follow up PR to not litter this one 👍
Add the necessary config, handlers, validators and implementations to allow signing new native USDC transactions, which use a slightly different contract ABI, but which works the same. Approval is now called Permit, and the permit is signed with different data.
The Hub changes that go with this are minimal: nimiq/hub@3504e77