Skip to content
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

Wrong computation of shares when undelegating (all) tokens #153

Open
tjanez opened this issue Nov 9, 2021 · 12 comments
Open

Wrong computation of shares when undelegating (all) tokens #153

tjanez opened this issue Nov 9, 2021 · 12 comments
Labels
bug Something isn't working p:1 Priority: core feature

Comments

@tjanez
Copy link
Member

tjanez commented Nov 9, 2021

When trying to undelegate all tokens for the oasis1qpv5usaqa0w2j4442lf6kgjvmg82eeg9kqzzrrqk account,
the wallet offered the option to undelegate ALL, i.e. 20363.96 ROSE:
image

Internally, it converted this ROSE amount to 17162010000000 shares (NOTE: The wallet wrongly states 17162.01 shares which are in fact giga shares), however, the account only had 17162007087366 shares, so the transaction failed with the insufficient balance error:

{
  "tx_hash": "840328b0327d81de570d0f3494a1bbb85d7ac1fea5dd6cebf017125666963e35",
  "method": "staking.ReclaimEscrow",
  "fee": {
    "gas": 1276
  },
  "body": {
    "account": "oasis1qpn83e8hm3gdhvpfv66xj3qsetkj3ulmkugmmxn3",
    "shares": "17162010000000"
  },
  "nonce": 10,
  "signature": {
    "signature": "7zIc9d1VDzKmcXQ38+sn3Zm7/pUesDp16TxdJ85zCzW4glCTCEsFpn0M3DyI8A7tUc4sgyLZWL5XSB01DN/wAA==",
    "public_key": "XqjcmqoKpTpbZ3eonAhipflyBk5liuABRAAry4Is4uA=",
    "address": "oasis1qpv5usaqa0w2j4442lf6kgjvmg82eeg9kqzzrrqk"
  },
  "height": 5833799,
  "timestamp": 1636379158,
  "time": "2021-11-08T13:45:58Z",
  "error": {
    "module": "unknown",
    "code": 1,
    "message": "insufficient balance"
  }
}

The work-around is to manually change the amount to something less (e.g. subtract 0.1 ROSE):
image (1)

In this case, the transaction went through:

{
  "tx_hash": "400c2e213e55d0e6d0371bdf90f768fb823bd24320e252613d20ec28c01f8af3",
  "method": "staking.ReclaimEscrow",
  "fee": {
    "gas": 1276
  },
  "body": {
    "account": "oasis1qpn83e8hm3gdhvpfv66xj3qsetkj3ulmkugmmxn3",
    "shares": "17162001500000"
  },
  "nonce": 11,
  "signature": {
    "signature": "EbLrzOMNJLhewSbmUow7LGH21HteyU1Apcu2d22uX3oNqYO+M2L4i09BEJckapm+AQeVs5lwe6aDSsurq25sDw==",
    "public_key": "XqjcmqoKpTpbZ3eonAhipflyBk5liuABRAAry4Is4uA=",
    "address": "oasis1qpv5usaqa0w2j4442lf6kgjvmg82eeg9kqzzrrqk"
  },
  "height": 5834113,
  "timestamp": 1636381020,
  "time": "2021-11-08T14:17:00Z",
  "error": {
    "code": 0
  }
}
@tjanez tjanez added bug Something isn't working p:0 Priority: High! bugs, address immediately labels Nov 9, 2021
@lukaw3d
Copy link
Member

lukaw3d commented Nov 9, 2021

Also check if insufficient balance error is reported as transaction: invalid nonce

@lvshaoping007
Copy link
Contributor

We have thought about how to fix this, which is to show the share and use the share when reclaim it. No matter if there is no conversion problem. But there is a problem with users’ understanding of share. Many users don’t seem to know much about share.

@lvshaoping007
Copy link
Contributor

about invalid nonce . i check nonce , find the nonce is correct ,there may caused by share not enough , but grpc throw nonce error
image

@pro-wh
Copy link
Contributor

pro-wh commented Dec 16, 2021

image

I tried doing altering the code to let me send a higher number of shares and to log any errors, and it looks like it first gives an 'insufficient balance' error. But there's also automatic retry in the code, which when it tries to resend the same transaction, the nonce doesn't change (as expected, since the retry is meant for network problems). That leads to some 'invalid nonce' errors on the subsequent attempts. And that seems to be correct too.

Some ideas:

  • maybe have some piece of code to recognize errors not related to network connectivity and abort the retry loop earlier
  • we currently leave the retry loop with the error from the last attempt. maybe that's not the best thing to do?

@lvshaoping007
Copy link
Contributor

if there have error , break the loop is a good idea . we should not try with add nonce ?
i think this is good idea . and how about share ? i saw there have some question about reclaim , this may caused by conversion between amount and share . i fix this in my local by change amount to share . Only when the share is entered, the estimated amount is displayed

@lvshaoping007
Copy link
Contributor

i make a fix in another branch, there just fix two issue
1, Use share as the main unit for reclaim
2, fix submit error . when get grpc error , there will reject error ,and other will go on .
you kan run in your local . this is the fix commit .
branch stakeShare
fix commit

@pro-wh
Copy link
Contributor

pro-wh commented Dec 17, 2021

Sorry for the complication, but I think our side would like to discuss whether or not we change over to the proposed UI where shares become the main unit. I'll see if we can come up with a recommended algorithm for converting a token amount to shares if we want to keep the UI the way it is.

@lvshaoping007
Copy link
Contributor

We have considered this situation, but since the amount is always changing, each block is different, and the only constant is share, so in the algorithm, you may also need to consider the issue of the amount changing with the block. This is why I finally switched to share as the unit

@tjanez tjanez added p:1 Priority: core feature and removed p:0 Priority: High! bugs, address immediately labels Jan 7, 2022
@dgutierr
Copy link

I tried to undelegate from both two validators. The transaction was cancelled with an "insufficient funds errors". I tried the work-around you mentioned of lowering the amount and now I get an "invalid nonce" error right from the oasis wallet extension. Im stucked and unable to undelegate my funds now. Any ideas how to fix that? thanks a lot.

@lvshaoping007
Copy link
Contributor

Is it working now? I tried it just now, undelegate is normal.
if there is a nonce error, you need to return to the previous page and re-enter, because the nonce is requested when entering the page

@dgutierr
Copy link

Finally, after a few hours, the staking funds were listed under unbounded delegations. The issue is basically the time between unstaking and confirmation as unbounded delegation. Within that period, if you unstake again you get the nonce error. I think an intermediate state as "unstaking approval pending confirmation" or similar would help :-)

@lvshaoping007
Copy link
Contributor

Ok , thanks for your advice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p:1 Priority: core feature
Projects
None yet
Development

No branches or pull requests

5 participants