Nft transfers #311
Merged
Nft transfers #311
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
NEW UI: * Tab-Navigation to fill out a CSV for Asset-Transfers or Collectible-Transfers * transfer-tables are now always displayed under the CSV-Form. * there are now two tables: For asset transfers and for collectible transfers * submit button is at the bottom of these tables issue #16
* one combined CSV file for nft / asset transfers * tables are wrapped in accordions
* Updates help text * Validates, that the value is a integer for erc1155 transfers * unittests for the transfer of collectibles * unittest for decimal (invalid) erc1155 * fixes sample file
bh2smith
approved these changes
Nov 30, 2021
| <code> | ||
| <b>token_address</b> | ||
| </code> | ||
| : Ethereum address of ERC20 token to be transferred. This has to be left blank for native (ETH) |
There was a problem hiding this comment.
Nit
Suggested change
| : Ethereum address of ERC20 token to be transferred. This has to be left blank for native (ETH) | |
| : Ethereum address of ERC20 token to be transferred. This must be left blank for native (ETH) |
| </Title> | ||
| <Text size="lg"> | ||
| Since native tokens do not have a token address, you must leave the <code>token_address</code> column | ||
| blank for native transfers. |
There was a problem hiding this comment.
Suggested change
| blank for native transfers. | |
| blank for native transfers. See the sample transfer file for an example of this. |
src/parser/transformation.ts
Outdated
| export const transform = ( | ||
| row: CSVRow, | ||
| tokenInfoProvider: TokenInfoProvider, | ||
| erc721InfoProvider: ERC721InfoProvider, |
Comment on lines
+12
to
+14
| case "erc1155": | ||
| case "erc721": | ||
| validateCollectibleRow(row, callback); |
There was a problem hiding this comment.
I am new to TS switches, but I'm guessing this means both cases are validated and not that one is skipped.
* instead of providing erc1155/erc721 the token_type now is simply nft * fixes performance problem of editor. For no reason the csvContent was held by the App and passed down to the editor * tests for nft transfers * updated faq
bh2smith
approved these changes
Dec 3, 2021
|
We should deploy a new version of this, get the ipfs hash and share it with the people interested in this feature. |
bh2smith
reviewed
May 3, 2022
| transfer.from, | ||
| transfer.receiver, | ||
| transfer.tokenId.toFixed(), | ||
| transfer.value?.toFixed() ?? "0", |
There was a problem hiding this comment.
I think the default here should be "1" and not "0". Because ERC1155 tokens have a balanceOf for a single ID.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After some work on this, the first version is done.
It includes:
Here is a transaction I submitted including erc20, erc1155 and native tokens:
https://rinkeby.etherscan.io/tx/0x158e3794c672ad550726477d5c12fa6cfc90c0fbddf0cffeb2062b07ee0b7f89
Screenshot:
I didn't try it on mainnet yet, because I don't have a safe there.