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

ProverPool bug (address gets removed) #14402

Closed
adaki2004 opened this issue Aug 7, 2023 · 9 comments · Fixed by #14411
Closed

ProverPool bug (address gets removed) #14402

adaki2004 opened this issue Aug 7, 2023 · 9 comments · Fixed by #14411

Comments

@adaki2004
Copy link
Contributor

adaki2004 commented Aug 7, 2023

Describe the bug

Ivy from ZkPool reached out to us, reporting that she staked with 2 addresses , 5 TKOe each. (0x08f0B99Ab6624bdf7bC4F0A75504a64154791fD8, 0x2EA93391Cd55617E8e4CA1FD66E881d5ACbAA76B).

She got assigned to blocks and she got slashed couple of times (but 0.01 - so the minimum amount) but around couple of minutes (cca. 30mins) she disappeared from the provers table. (top32).

Her stake with 0x2e.. can be seen here:
https://explorer.test.taiko.xyz/tx/0xf6c3b14385f59ae747fa54c5114db4497c364ec9074c9ec711f9c31defb0a3b1

I created a python script which parses events (from the (block number of the staking) - 1) in which this 0x2e.. address is mentioned , see the code in the additional context section together with the log.

The weird thing is, since she staked there are 4 events:
1 stake
1 exit (which comes with the stake by default)
2 slashes

Nothing more, but she is not in the prover table any more.
If you check the explorer:
https://explorer.test.taiko.xyz/address/0xC9580414A4372BDdBd8e19e01854DC0B2b1390Cf/read-proxy#address-tabs

She is not there. Altough when you query getStaker("0x2EA93391Cd55617E8e4CA1FD66E881d5ACbAA76B") you get this:

[
staker (tuple[uint64,uint64,uint32,uint32]) : 
exitRequestedAt (uint64) : 0
exitAmount (uint64) : 0
maxCapacity (uint32) : 32
proverId (uint32) : 3

prover (tuple[uint64,uint32,uint32]) : 
stakedAmount (uint64) : 0
rewardPerGas (uint32) : 1
currentCapacity (uint32) : 0
]

UPDATE (to this above tuple): When you query the result now, you will see different prover struct. This is because
address 0xae3f94f9d84dd8aff257c6bb9a5a8e61d930162a came in to provers (occupiying the place proverId:3 represents, so when you query Ivy's address, - since it points to the porverId: 3, it will print those results)

Since she is not in the prover table, her proverId shall not be 3.. Because we can clearly see she is not in the getProvers().

Steps to reproduce

Steps to reproduce here.

Additional context

Log:

It is an exit by this address
{'address': '0xc9580414a4372bddbd8e19e01854dc0b2b1390cf', 'topics': ['0x7b870040d0137f84191e3e446a10f48b5ac5d26ec96be3f795fcfc4c954410fe', '0x0000000000000000000000002ea93391cd55617e8e4ca1fd66e881d5acbaa76b'], 'data': '0x00000000000000000000000000000000000000000000000000000000153f0d70', 'blockNumber': '0xfc3d6', 'transactionHash': '0xf6c3b14385f59ae747fa54c5114db4497c364ec9074c9ec711f9c31defb0a3b1', 'transactionIndex': '0x5', 'blockHash': '0x2db671107c9a942c244b6c00e4f1190b5f7a04e803681b68ec5292def9b6d853', 'logIndex': '0xd', 'removed': False}
It is a stake this address
{'address': '0xc9580414a4372bddbd8e19e01854dc0b2b1390cf', 'topics': ['0x5ca6ec890c0c084d4fe6c6c49e6aea6fd8dbf1460730c83b5b12bf22811851e3', '0x0000000000000000000000002ea93391cd55617e8e4ca1fd66e881d5acbaa76b'], 'data': '0x000000000000000000000000000000000000000000000000000000001dcd650000000000000000000000000000000000000000000000000000000000000027100000000000000000000000000000000000000000000000000000000000000020', 'blockNumber': '0xfc3d6', 'transactionHash': '0xf6c3b14385f59ae747fa54c5114db4497c364ec9074c9ec711f9c31defb0a3b1', 'transactionIndex': '0x5', 'blockHash': '0x2db671107c9a942c244b6c00e4f1190b5f7a04e803681b68ec5292def9b6d853', 'logIndex': '0x11', 'removed': False}
It is a slash by this address
{'address': '0xc9580414a4372bddbd8e19e01854dc0b2b1390cf', 'topics': ['0xdd80bbe216163c1792fa59b50e56f1a7ac79674c4815b65da0ef875a39655e08', '0x0000000000000000000000002ea93391cd55617e8e4ca1fd66e881d5acbaa76b'], 'data': '0x000000000000000000000000000000000000000000000000000000000004e200', 'blockNumber': '0xfd12a', 'transactionHash': '0x6e89cd1fdc9737f438e460726e4a176e2f976a6c8ffe98bb2517ff2c238d41e4', 'transactionIndex': '0x2', 'blockHash': '0x7bc638f65fee956ad4f6e2ed65ba9dfac1c1715084290b333555aa7a58be2f6a', 'logIndex': '0x8', 'removed': False}
It is a slash by this address
{'address': '0xc9580414a4372bddbd8e19e01854dc0b2b1390cf', 'topics': ['0xdd80bbe216163c1792fa59b50e56f1a7ac79674c4815b65da0ef875a39655e08', '0x0000000000000000000000002ea93391cd55617e8e4ca1fd66e881d5acbaa76b'], 'data': '0x00000000000000000000000000000000000000000000000000000000003f8788', 'blockNumber': '0xfda56', 'transactionHash': '0xf5fe8086f01c25acb6407c7298c9b8d4ef72d420c9546c2758f42814b2d616fc', 'transactionIndex': '0x1', 'blockHash': '0x75f976efcbf5536c3a30d835d95040156d4a4d87ab32a760d92e0b0613b8b887', 'logIndex': '0x4', 'removed': False}

Simply run it as python3 script.py (once you set the RPC URL and installed the requests lib)

import requests
import json

# L2 logs
url = "USE_AN_L2_RPC_ENDPOINT_URL" 

slashed_hash = "dd80bbe216163c1792fa59b50e56f1a7ac79674c4815b65da0ef875a39655e08"
staked_hash = "5ca6ec890c0c084d4fe6c6c49e6aea6fd8dbf1460730c83b5b12bf22811851e3"
exited_hash = "7b870040d0137f84191e3e446a10f48b5ac5d26ec96be3f795fcfc4c954410fe"
withdrawn_hash = "bae95d59332d6e1e8f1ae78e7bebdaeef072d57b731c8790a636667e3a0a87ee"

# Set these params
address_to_query = "2ea93391cd55617e8e4ca1fd66e881d5acbaa76b"
block_nr_to_query_from = "0xFC3D5"

payload = json.dumps({
  "method": "eth_getLogs",
  "params": [
    {
    "address": '0xC9580414A4372BDdBd8e19e01854DC0B2b1390Cf', 
    "fromBlock": block_nr_to_query_from
    }
  ],
  "id": 1,
  "jsonrpc": "2.0"
})
headers = {
  'Content-Type': 'application/json'
}

response = requests.request("POST", url, headers=headers, data=payload)
jsonResponse = response.json() 
for item in jsonResponse['result']:
        if address_to_query in str(item):
          if staked_hash in str(item):
            print("It is a stake this address")
            print(item)
        
          if exited_hash in str(item):
            print("It is an exit by this address")
            print(item)
          
          if slashed_hash in str(item):
            print("It is a slash by this address")
            print(item)
                    
          if withdrawn_hash in str(item):
            print("It is a withdrawal by this address")
            print(item)
          
@adaki2004 adaki2004 changed the title ProverPool bug (addresses gets removed (?)) ProverPool bug (address gets removed (?)) Aug 7, 2023
@dantaik
Copy link
Contributor

dantaik commented Aug 7, 2023

@davidtaikocha could you work on this one?

@davidtaikocha
Copy link
Member

@davidtaikocha could you work on this one?

Yep sure, will do the investigation

@adaki2004
Copy link
Contributor Author

adaki2004 commented Aug 7, 2023

+1 possible clue (activity around this address): 0xae3f94f9d84dd8aff257c6bb9a5a8e61d930162a

The reason it is suspicious because if we query getStaker() with this address, it also says proverId: 3 - same as Ivy's 0x2EA93391Cd55617E8e4CA1FD66E881d5ACbAA76B.

(UPDATE: This address staked well after Ivy got kicked out, so might be not related at all! But given Ivy has still proverId 3, suspicious on it's own - how Ivy's address could retain proverId: 3 (one possible scneario below, but we should still saw the Exit.. .)

@adaki2004
Copy link
Contributor Author

adaki2004 commented Aug 7, 2023

@dantaik @davidtaikocha one scenario to think about:

  1. Let's say Alice is in the top32 - but slashed down to 0. Now Bob comes in and stakes, which will trigger a 'replaced' transaction (replaced exit). But in _exit() we have this check, that only in case we clear data for staker:
    if (prover.stakedAmount > 0) {
    So what happens is, that Alice tho replaced within the provers, she still has a proverId.
    I dont think this is the cause of the issue, because we should have seen an Exit event for Alice, but still an issue (imo).

@adaki2004
Copy link
Contributor Author

adaki2004 commented Aug 7, 2023

Okay, so i have an explanation (tho i'd like to request more pair of eyes on it because it is just one possible answer how it could happen).

David sent me a hint, that address: 0x5A0C30dd34d0a05742c64725D44505607e6Fbee4 was able to withdraw Ivy's funds from the prover pool and it seemed that when he/she staked she exited his/her funds but actually it was Ivy's fund.
kép

Let me explain how it could happen.
Let's say the top prover pool is just 5 not 32.

We have stakers:

1. Alice 100 TKO
2. Bob 99 TKO
3. Cecile 21 TKO
4. David 18 TKO
5. Ed 10 TKO

Let's say we have lots and lots of slashed (as how it happened last night see David's post here.)

So that Ed is slashed to 0. Now the new proving table is the following:

1. Alice 100 TKO
2. Bob 99 TKO
3. Cecile 21 TKO
4. David 18 TKO
5. Ed 0 TKO

Now, Fred wants to stake and he is indeed able to stake on position 5 - force-exiting Ed. But since the previous comment/bug highlights, Ed actually could retain his proverId: 5 because we only clear his value in case if (prover.stakedAmount > 0) soo.. when Ed re-stakes the code actually thinks his proverId = 5 but it is (should) not true.
So actually when he re-stakes, the protocol _exit() Fran's fund thinking it's for Ed, but not..

I cannot say exactly if this happaned, but i'm pretty sure - it would be cool to go back and see 0x5A0C30dd34d0a05742c64725D44505607e6Fbee4's position was proverId:3 before Ivy staked, because when i query the events of this address it has tons of slashed ones.

Imo prover pool contract got a bit too complex if debugging this is issue seems to be very challenging for 2 engineers for several hours.

@ivy-aoraki-labs
Copy link
Contributor

Not sure if this helps, but a strange thing I noticed yesterday is :

image

Look, the connected wallet address is mine 0x2EA...A768, but the address shown in the middle card is someone else'

@adaki2004
Copy link
Contributor Author

adaki2004 commented Aug 8, 2023

@dantaik @davidtaikocha

I think an easy fix is the following, in _exit() function. So basically when !checkExitTimestamp it means we need a 'replace' -> if we need a replace then we need to clear proverId for that same staker.

if (prover.stakedAmount > 0 || !checkExitTimestamp ) {
           ...
       }

@davidtaikocha
Copy link
Member

davidtaikocha commented Aug 8, 2023

@dantaik @davidtaikocha

I think an easy fix is the following, in _exit() function. So basically when !checkExitTimestamp it means we need a 'replace' -> if we need a replace then we need to clear proverId for that same staker.

if (prover.stakedAmount > 0 || !checkExitTimestamp ) {
           ...
       }

Yep this fix makes some sense to me (+ left a comment in #14411 ), and we also need to clean those dirty data?

@adaki2004 adaki2004 changed the title ProverPool bug (address gets removed (?)) ProverPool bug (address gets removed) Aug 8, 2023
@dantaik
Copy link
Contributor

dantaik commented Aug 9, 2023

Thank you, @adaki2004, for analyzing and fixing the bug. Lets verify the fix with a upgrade to Eldfell L3. @davidtaikocha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants