Add method: transfernft & show nft #304

chenzhitong · 2023-11-07T07:08:25Z

close #193

transfernft

Usage: neoxp transfernft [options] <Contract> <TokenId> <Sender> <Receiver>

Arguments:
  Contract                  NFT Contract (symbol or script hash)
  TokenId                   TokenId of NFT (Base64 string)
  Sender                    Account to send NFT from
  Receiver                  Account to send NFT to

Options:
  -d|--data <DATA>          Optional data parameter to pass to transfer operation
  -p|--password <PASSWORD>  password to use for NEP-2/NEP-6 sender
  -i|--input <INPUT>        Path to neo-express data file
  -t|--trace                Enable contract execution tracing
  -j|--json                 Output as JSON
  -?|-h|--help              Show help information.

show nft

Usage: neoxp show nft [options] <Contract> <Account>

Arguments:
  Contract            Contract to show NFT of (symbol or script hash)
  Account             Account to show asset balance for

Options:
  -i|--input <INPUT>  Path to neo-express data file
  -?|-h|--help        Show help information.

sync

cschuchardt88 · 2023-11-07T07:57:46Z

src/neoxp/Commands/TransferNFTCommand.cs

+                var (chainManager, _) = chainManagerFactory.LoadChain(Input);
+                var password = chainManager.Chain.ResolvePassword(Sender, Password);
+                using var txExec = txExecutorFactory.Create(chainManager, Trace, Json);
+                await txExec.TransferNFTAsync(Contract, Encoding.UTF8.GetString(Convert.FromBase64String(TokenId)), Sender, password, Receiver, Data).ConfigureAwait(false);


TokenId Should be in hex. This is the way it is done in production. Will be easier to understand.

cschuchardt88 · 2023-11-07T07:59:17Z

src/neoxp/Commands/TransferNFTCommand.cs

+        internal string Data { get; init; } = string.Empty;
+
+        [Option(Description = "password to use for NEP-2/NEP-6 sender")]
+        internal string Password { get; init; } = string.Empty;


NEP-17 transfers dont require password. Seeing how there is default.neo-express file that holds it.

This is copied from TransferCommand.cs and is the same as NEP-17.

yes but for password to use for NEP-2/NEP-6 sender for legacy i believe.

Also keep in mind for batch it doesn't require password for transfers. That's what i was thinking of.

cschuchardt88 · 2023-11-07T08:02:10Z

src/neoxp/neoxp.csproj

+  <ItemGroup>
+    <PackageReference Update="Neo.BlockchainToolkit.Library" Version="3.6.19" />
+  </ItemGroup>
+


3.6.19 doesn't exists anymore its 3.6.0 now. But we have updated repo with lots of changes please merge before adding this. If still needed then keep it.

cschuchardt88 · 2023-11-07T08:03:48Z

src/neoxp/Properties/launchSettings.json

+      "commandLineArgs": "run"
+    }
+  }
+}


Please dont merge your launchSettings.json.

cschuchardt88

Dont forget to run dotnet format and your analyzers to fix github actions.

Edit:
https://github.com/cschuchardt88/neo-construct/blob/master/src/NeoEvents.Construct/Node/Tokens/Nep11Token.cs is good resource for adding the methods for NEP-11, But i dont remember the proper way to invoke the VM in neo-express.

into transfernft

cschuchardt88 · 2023-11-07T08:18:34Z

src/neoxp/Commands/TransferNFTCommand.cs

+                }
+                else
+                {
+                    return Encoding.UTF8.GetString(hex[2..].HexToBytes().Reverse().ToArray());


Convert.FromHexString or Convert.ToHexString is good too.

cschuchardt88 · 2023-11-07T11:45:01Z

since you are working on #193 can you also checkout and think adding NEP-11 for #194

cschuchardt88 · 2023-11-09T02:40:50Z

You have forgotten about Divisible vs Non-Divisible NFTs

Please see
https://github.com/neo-project/proposals/blob/master/nep-11.mediawiki#user-content-transfer-2

.vs
https://github.com/neo-project/proposals/blob/master/nep-11.mediawiki#user-content-transfer

cschuchardt88 · 2023-11-09T02:45:58Z

src/neoxp/Extensions/ExpressNodeExtensions.cs

+            }
+            catch (Exception)
+            {
+                throw new Exception("invalid script results");


Can we get a better error output? Something indicating that the contract isn't an NFT or missing a method

chenzhitong · 2023-11-09T03:13:12Z

You have forgotten about Divisible vs Non-Divisible NFTs

Please see https://github.com/neo-project/proposals/blob/master/nep-11.mediawiki#user-content-transfer-2

.vs https://github.com/neo-project/proposals/blob/master/nep-11.mediawiki#user-content-transfer

Template for neo-devpack-dotnet only has base class for Non-divisible NFT, so, I think just supporting Non-divisible NFT is enough.

BTW, does any wallet support Divisible NFT now?

cschuchardt88 · 2023-11-09T03:24:17Z

BTW, does any wallet support Divisible NFT now?

I believe NeoLine. That what the Number 1 is for? But I don't think we should limit it.

Template for neo-devpack-dotnet only has base class for Non-divisible NFT, so, I think just supporting Non-divisible NFT is enough.

Once we add the validator for NEP-11 this won't be a problem. That is because in neo-devpack-dotnet if you want divisible you just have to add these functions transfer ownerOf and balanceOf. Also seeing how the class is abstract the non-divisible ones won't be on the interface.

chenzhitong · 2023-11-09T03:31:14Z

This count is to the number of NFTs held in the same contract
When you click on it, you will see the details

chenzhitong · 2023-11-09T08:42:50Z

Who can review it

Jim8y · 2023-11-09T08:52:28Z

Lacks unit test, all i can do is merge when you think its good to go. If you think it is good to go, i can merge it for you

chenzhitong · 2023-11-09T08:57:59Z

I've tested it. Merge it please. @Liaojinghui

Jim8y · 2023-11-09T09:01:02Z

@chenzhitong @cschuchardt88 @Ashuaidehao Can you add unit test to this repo? I remember that @cschuchardt88 you wanted to add something that contains the unit test to this repo, right? I think we can just make things easy.

cschuchardt88 · 2023-11-09T17:12:25Z

@chenzhitong @cschuchardt88 @Ashuaidehao Can you add unit test to this repo? I remember that @cschuchardt88 you wanted to add something that contains the unit test to this repo, right? I think we can just make things easy.

I wanted to add neo-blockchaintoolkit-library to this repo that's where all the unit tests are for core functionally.

@chenzhitong
can you make sure you put file header in *.cs files please.

In Visual Studio set it up like this

Than you can run like so if needed:

cschuchardt88 · 2023-11-15T14:00:08Z

@chenzhitong can you reopen and fix?

cschuchardt88 · 2023-11-15T14:29:25Z

Add TokenIds as base64 as well
Fix error output to show inter error
Make wording say [Argument(1, Description = "TokenId of NFT (Format: HEX, BASE64)")]
Make wording say [Argument(0, Description = "NFT Contract (Symbol or Script Hash)")]
Add transfernft command to batch commands
Allow contract names

Try not to use Regex makes it hard to quickly understand; unless it a very lengthy task to validate. Why you use Reverse when you have 0x?

Fix show nft

C:\Users\edged\Code\sott>neoxp batch run.batch
Transfer Transaction 0xa9096bc8e3d24a8bf264bc8801bf3601c6e8040a3bf1a63e21b4e9d498945926 submitted
Deployment of sword (0x8b49d75870e876b610b430e3db108d487d545dd1) Transaction 0x4a67944d49fb6ae9bf7667c5419495188de2bad98f032ead24fca3adb5013c8c submitted
Invocation Transaction 0x6750b0223adc16659fb6436e5755604ad71b27fe8c30664e372e6316fda0a36b submitted
Invocation Transaction 0x7493007e207525bd5e93154c758284ea334eee377436be2485e3933e495aa19a submitted

C:\Users\edged\Code\sott>neoxp show nft -h
Show NFT Tokens for account

Usage: neoxp show nft [options] <Contract> <Account>

Arguments:
  Contract            Contract to show NFT of (symbol or script hash)
  Account             Account to show asset balance for

Options:
  -i|--input <INPUT>  Path to neo-express data file
  -?|-h|--help        Show help information.


C:\Users\edged\Code\sott>
C:\Users\edged\Code\sott>neoxp show nft SOTT bob
System.ArgumentException: Unknown Asset "SOTT" (Parameter 'asset')

C:\Users\edged\Code\sott>neoxp show nft sword bob
System.ArgumentException: Unknown Asset "sword" (Parameter 'asset')

C:\Users\edged\Code\sott>neoxp show nft 0x8b49d75870e876b610b430e3db108d487d545dd1 bob
System.Exception: invalid script results

chenzhitong and others added 5 commits November 3, 2023 15:25

Merge pull request #1 from neo-project/master

c36e685

sync

temp

061e782

TransferNFT

758513c

update

168e9fb

Merge branch 'master' into transfernft

0ea8428

cschuchardt88 reviewed Nov 7, 2023

View reviewed changes

src/neoxp/Properties/launchSettings.json Outdated

"commandLineArgs": "run"

}

}

}

Copy link

Member

cschuchardt88 Nov 7, 2023

Choose a reason for hiding this comment

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

Please dont merge your launchSettings.json.

cschuchardt88 reviewed Nov 7, 2023

View reviewed changes

chenzhitong added 3 commits November 7, 2023 16:14

hex string

419ef60

Merge branch 'transfernft' of https://github.com/chenzhitong/neo-express

7ec1c31

into transfernft

update

8a79bf0

cschuchardt88 reviewed Nov 7, 2023

View reviewed changes

chenzhitong added 2 commits November 7, 2023 16:23

format

2c826b3

format

116c863

chenzhitong added 2 commits November 8, 2023 15:45

show nft

bf098f2

format

4a54cb8

chenzhitong changed the title ~~Add method transfernft~~ Add method: transfernft & show nft Nov 8, 2023

chenzhitong marked this pull request as ready for review November 8, 2023 08:02

Merge branch 'master' into transfernft

8bac95d

cschuchardt88 reviewed Nov 9, 2023

View reviewed changes

Merge branch 'master' into transfernft

b3de7c2

Merge branch 'master' into transfernft

e824ae9

Merge branch 'master' into transfernft

da1d53e

Jim8y approved these changes Nov 9, 2023

View reviewed changes

Jim8y merged commit e65d7c4 into neo-project:master Nov 9, 2023

cschuchardt88 mentioned this pull request Nov 15, 2023

Preview review v3.6.40 - Reports #338

Closed

22 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add method: transfernft & show nft #304

Add method: transfernft & show nft #304

chenzhitong commented Nov 7, 2023 •

edited

Loading

cschuchardt88 Nov 7, 2023

cschuchardt88 Nov 7, 2023

chenzhitong Nov 7, 2023

cschuchardt88 Nov 7, 2023

cschuchardt88 Nov 7, 2023

cschuchardt88 Nov 7, 2023 •

edited

Loading

cschuchardt88 Nov 7, 2023

cschuchardt88 left a comment •

edited

Loading

cschuchardt88 Nov 7, 2023

cschuchardt88 commented Nov 7, 2023

cschuchardt88 commented Nov 9, 2023 •

edited

Loading

cschuchardt88 Nov 9, 2023

chenzhitong commented Nov 9, 2023

cschuchardt88 commented Nov 9, 2023 •

edited

Loading

chenzhitong commented Nov 9, 2023

chenzhitong commented Nov 9, 2023

Jim8y commented Nov 9, 2023 •

edited

Loading

chenzhitong commented Nov 9, 2023

Jim8y commented Nov 9, 2023

cschuchardt88 commented Nov 9, 2023

cschuchardt88 commented Nov 15, 2023 •

edited

Loading

cschuchardt88 commented Nov 15, 2023 •

edited

Loading

Add method: transfernft & show nft #304

Add method: transfernft & show nft #304

Conversation

chenzhitong commented Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cschuchardt88 Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cschuchardt88 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cschuchardt88 commented Nov 7, 2023

cschuchardt88 commented Nov 9, 2023 • edited Loading

Choose a reason for hiding this comment

chenzhitong commented Nov 9, 2023

cschuchardt88 commented Nov 9, 2023 • edited Loading

chenzhitong commented Nov 9, 2023

chenzhitong commented Nov 9, 2023

Jim8y commented Nov 9, 2023 • edited Loading

chenzhitong commented Nov 9, 2023

Jim8y commented Nov 9, 2023

cschuchardt88 commented Nov 9, 2023

cschuchardt88 commented Nov 15, 2023 • edited Loading

cschuchardt88 commented Nov 15, 2023 • edited Loading

chenzhitong commented Nov 7, 2023 •

edited

Loading

cschuchardt88 Nov 7, 2023 •

edited

Loading

cschuchardt88 left a comment •

edited

Loading

cschuchardt88 commented Nov 9, 2023 •

edited

Loading

cschuchardt88 commented Nov 9, 2023 •

edited

Loading

Jim8y commented Nov 9, 2023 •

edited

Loading

cschuchardt88 commented Nov 15, 2023 •

edited

Loading

cschuchardt88 commented Nov 15, 2023 •

edited

Loading