navcoin / navcoin-core

bitcoin-core 0.13 fork ported for NavCoin
MIT License
123 stars 92 forks source link

Added is_mine property to listtokens return value if token balance is… #930

Closed mxaddict closed 2 years ago

mxaddict commented 2 years ago

closes #923

aguycalled commented 2 years ago

i think what sakdeniz meant with this request was to filter the entries which are owned by the wallet (independently of the balance). you can check the logic at the minttoken or mintnft rpc commands to see how to check if a token is owned (line 1622 rpcwallet.cpp for example)

mxaddict commented 2 years ago

I see, should the (mine) param for listtokens be updated as well? Cause that shows tokens that have balance, but might not be owned by the wallet.

aguycalled commented 2 years ago

yes probably, those tokens can easily be filtered without the current mine just checking for balance > 0 client side

mxaddict commented 2 years ago

yes probably, those tokens can easily be filtered without the current mine just checking for balance > 0 client side

Alright

mxaddict commented 2 years ago

@aguycalled I've updated the logic, does this look good?

aguycalled commented 2 years ago

i would need to test it, but initially it looks to me like it would label as mine tokens which are not owned by the wallet

navbuilder commented 2 years ago

A new build of 8ead11a00555b7e5cff2d2c9747141631a1e1537 has completed succesfully! Binaries available at https://build.nav.community/binaries/add-is-mine-to-listtokens-rpc-command

navbuilder commented 2 years ago

A new build of 2d7e4345f919e7b6a5fa87a7f63aa84d257cf153 has completed succesfully! Binaries available at https://build.nav.community/binaries/add-is-mine-to-listtokens-rpc-command

mxaddict commented 2 years ago

@aguycalled this seems to be working with my testing but @sakdeniz said he was having issues, I'm still trying to investigate.

@sakdeniz could you let me know what steps you took so I can replicate your issue with this.

aguycalled commented 2 years ago

from what I read in the code

             // Is this token ours?
             bool fTokenIsMine = true;

isMine is by default yes

             if (!pwalletMain->HaveBLSCTTokenKey(it->second.key))
             {

this block runs only if the wallet does not have the key for the token

                 if (pk.GetG1Element() != it->second.key)
                     fTokenIsMine = false;

wallet compares the stored key with the assigned to the token.

in my opinion it should be false by default and set it to true if the stored key equals the assigned to the token

but giving this opinion just from reading the code

mxaddict commented 2 years ago

Does this code return true even if the spend key does not own the token/nft?

pwalletMain->HaveBLSCTTokenKey(it->second.key)
mxaddict commented 2 years ago

If it returns true when spendkey does not own the token or nft, I think we will need to update the logic in mintnft and minttoken as well.

mxaddict commented 2 years ago

@aguycalled this seems to be working with my testing but @sakdeniz said he was having issues, I'm still trying to investigate.

@sakdeniz could you let me know what steps you took so I can replicate your issue with this.

@sakdeniz I think you might have been running the first build (Which was only checking balance)

navbuilder commented 2 years ago

A new build of a9cafefed1c22a87042b738c4eaebfb53d8efa12 has completed succesfully! Binaries available at https://build.nav.community/binaries/add-is-mine-to-listtokens-rpc-command

chasingkirkjufell commented 2 years ago

just wondering, why not add a filter for the rpc instead? like listtokens mine wouldn't that be more straight forward and the output will be clean instead of trying to go through all of them and find the one has is_mine: true

sakdeniz commented 2 years ago

Tested with latest build, and it works fine.

navbuilder commented 2 years ago

A new build of fb3804b21c9dd2c314cb5f1a3cb188396964fd6a has completed succesfully! Binaries available at https://build.nav.community/binaries/add-is-mine-to-listtokens-rpc-command