filecoin-project / lotus

Reference implementation of the Filecoin protocol, written in Go
https://lotus.filecoin.io/
Other
2.85k stars 1.27k forks source link

Lotus-wallet bugs #8459

Open s0nik42 opened 2 years ago

s0nik42 commented 2 years ago

Checklist

Lotus component

Lotus Version

Daemon:  1.14.1+calibnet+git.d2dca7b21+api1.5.0
Local: lotus version 1.14.1+calibnet+git.d2dca7b21

Lotus-wallet : 
lotus-wallet version 1.14.1+mainnet

Describe the Bug

Hello,

I did a lot of test on lotus-wallet and here is the list of bugs I'm facing.

  1. Unable to sign ChangeOwnerAddress message (using a ledger). This is really annoying has this is the main reason to use it (in my use case)

    2022-04-08T00:22:00.591+0200    WARN    rpc go-jsonrpc@v0.1.5/handler.go:279    error in RPC call to 'Filecoin.WalletSign': Unexpected method
  2. When compiling on calibnet version show as mainnet:

    lotus-wallet version 1.14.1+mainnet
  3. When creating a new address the ledger (through its menu) doesn't provide the option to cancel the address creation. The only possible action is "Accept Address" . Not sure you can do something on that.

lotus wallet new secp256k1-ledger


4. When creating an new address if the ledger is connected but locked. Lotus-wallet doesn't wait and immediately generate the following message (might also show with other type of messages). In a more general manner, when lotus-wallet is waiting for the ledger to be ready we should have a message on screen  : 

2022-04-08T00:30:46.657+0200 WARN rpc go-jsonrpc@v0.1.5/handler.go:279 error in RPC call to 'Filecoin.WalletNew': getting public key from ledger: github.com/filecoin-project/lotus/chain/wallet/ledger.LedgerWallet.WalletNew /opt/lotus/src/lotus/chain/wallet/ledger/ledger.go:209

  1. Its not documented that FULLNODE_API_INFO has to be set for lotus-wallet to work without --offline option. either this should be documented, or default to wss://chain.luv ?

  2. Impossible do send to a new address (already reported #6368) . BUT It's possible using --offline argument. The error is linked to the inability for lotus-wallet to identify the actor to decoding the message for non-existing address. Might implement a fallback method when this happen.

  3. When lotus-wallet is not available any WalletList API call to the lotus node are failing (same as the lotus wallet list). At least the method shall return the local keys. :

    ERROR: RPC client error: sendRequest failed: Post "http://192.168.42.45:1777/rpc/v0": dial tcp 192.168.42.45:1777: connect: connection refused

I really see value for lotus-wallet and will be happy to support you @magik6k ?!? for testing. If you need more inforamtion let me know.

Logging Information

N/A

Repo Steps

N/A

rjan90 commented 2 years ago

Hey @s0nik42!

Thanks for the comprehensive report about lotus-wallet. I have added some extra labels and will assign it to the right team for analysis.

s0nik42 commented 2 years ago

Hello @rjan90 shall I keep my lotus-wallet setup up and running on my end ? or can I clean it ? depending the time frame on your side.
Cheers

rjan90 commented 2 years ago

Hey @s0nik42!

No need to keep it on your end if you do not want to. It should be fairly easy to replicate these issues. Thanks again for the detailed report.

s0nik42 commented 2 years ago

👍🏻

rjan90 commented 2 years ago

For 3. option to cancel the address creation it seems like the ledger has a option to reject the wallet creation. It is at the last page, see the pictures posted. IMG_7697 IMG_7698 IMG_7699 IMG_7700

rjan90 commented 2 years ago

For 1. ChangeOwnerAddress, I was not able to reproduce this on a local-network, and changing the owner-address of the storage provider worked without any issues:

First, changing the owner-address to a ledger:

./lotus-miner actor set-owner --really-do-it t1ybjdj6yh3cb6ltslejecyx7gosgngjwv6ld2nwi t3vstrf3apc4nbb32kpmlf3cvto6koiub2jolew4k3bolz74zpf2mnsxmnovb4ehtdzicnnfnjqxsjpekhyyla
Message CID: bafy2bzacecoa5on3dwbbntpyfr4ecph7q2kgnycrgiaoosi6toxxo4ywrjnx2
message succeeded!

And confirming the new owner-address:

./lotus-miner actor set-owner --really-do-it t1ybjdj6yh3cb6ltslejecyx7gosgngjwv6ld2nwi t1ybjdj6yh3cb6ltslejecyx7gosgngjwv6ld2nwi                                             
Message CID: bafy2bzacednjcqq5wzvo3mns36ksago66alixc245ckjzs3whsgvmt44qkzds
message succeeded!

Confirm that it has changed:

./lotus-miner actor control list                                                                                                                                             
name    ID      key           use    balance                          
owner   t01008  t1ybjdj6y...         9.999999369855003649 FIL         
worker  t0100   t3vstrf3a...  other  49999989.998919368601318749 FIL

Can you make sure that you are on the newest Filecoin-ledger app version?

rjan90 commented 2 years ago

So for now these seems to have been fixed (unchecked has not been fixed):

s0nik42 commented 2 years ago

Thank you, which version of lotus to get the fixes ?

rjan90 commented 2 years ago

@s0nik42 The tests where done on lotus version 1.15.1-dev+2k+git.4c94ae45d, but I think most of these could be related to fixes in the Ledger-Filecoin software, so make sure that you have the newest version on the Ledger (I was on v0.22.2 when I tested) 😄

f8-ptrk commented 2 years ago

For 3. option to cancel the address creation it seems like the ledger has a option to reject the wallet creation. It is at the last page, see the pictures posted. IMG_7697 IMG_7698 IMG_7699 IMG_7700

thats not how BIP39 style stuff works. addresses do not get created - they are already there

rjan90 commented 2 years ago

hats not how BIP39 style stuff works. addresses do not get created - they are already there

Semantics / Typo. The issue/ question was if it was possible to cancel or reject the message. Not how BIP39 works

ribasushi commented 2 years ago

@rjan90 a lender group that @TippyFlitsUK and I were trying to assist ran into problems trying to execute the steps https://github.com/filecoin-project/lotus/issues/8459#issuecomment-1116592038 without having a running lotus-miner. It appears that the lotus-shed code is different and does not yet support ledger. These codepaths should be unified to support the case of an owner who will never have access to a running lotus-miner system, which is 99% of the current cases for mucking with owner ( and/or beneficiary in the future )

rjan90 commented 2 years ago

Hey @ribasushi! So I dived a bit into the potential issue of not being able to change the owner using lotus-shed when using a Ledger device, but I was not able to reproduce it here on a local network.

Here where the steps I executed:

./lotus wallet list
Address                                                                                 Balance                         Nonce  Default  
t1ybjdj6yh3cb6ltslejecyx7gosgngjwv6ld2nwi                                               9.979998899955180538 FIL        5               
t3qcixsxivj5ygpqhi2ima526btslwexuzwl2urlto4wweszdkb4cs6bq65bwcsw7uvfpvmhxbn4norpne6zza  49999990.01974798121207203 FIL  15     X        

First, changing the owner-address to a ledger using lotus-shed:

phi:~$./lotus-shed actor set-owner --actor t01000 --really-do-it f1ybjdj6yh3cb6ltslejecyx7gosgngjwv6ld2nwi f3qcixsxivj5ygpqhi2ima526btslwexuzwl2urlto4wweszdkb4cs6bq65bwcsw7uvfpvmhxbn4norpne6zza
Message CID: bafy2bzacebloucxs5eqnujg2jsfnuzhnvndz3fgxqrbl7ky5g7bqv3ymyrjfe
message succeeded!

Confirm new owner using lotus-shed:

phi:~$./lotus-shed actor set-owner --actor t01000 --really-do-it f1ybjdj6yh3cb6ltslejecyx7gosgngjwv6ld2nwi f1ybjdj6yh3cb6ltslejecyx7gosgngjwv6ld2nwi                                             
Message CID: bafy2bzacebqlpwsfyb543vh4xscgdydwabx3kjyziwsfkf2iflybvhzftzohy
message succeeded!

Check that the owner changed on the lotus-miner side:

phi:~$./lotus-miner actor control list                                                                                                                                                           
name    ID      key           use    balance                         
owner   t01001  t1ybjdj6y...         9.979998899955180538 FIL        
worker  t0100   t3qcixsxi...  other  49999990.01974798121207203 FIL

Do we know what kind of error message they met? It might shed some more information why they failed to change the owner. But from my understanding and testing, changing a owner-adress with lotus-shed using a Ledger should not be an issue.