0xPolygonHermez / cdk-erigon

Ethereum implementation on the efficiency frontier
GNU Lesser General Public License v3.0
30 stars 24 forks source link

Counters undercounts on Bali #827

Closed mandrigin closed 3 weeks ago

mandrigin commented 1 month ago

seems like on Eth transfers, at least my load tests with eth transfers are causing that

image
mandrigin commented 1 month ago

(1) it is Bali network

(2) these batches are sent to L1 because the executor didn't ran into OOC, we still fit n the counter limit, just our local values are smaller

(3) I see these warnings after I just did normal eth transfers

mandrigin commented 1 month ago

@ignasirv here is an undershoot on 10 eth transfers I just reproduced

{
batch      54938
counter   P
erigon     68334
legacy    119080
t              2024-07-19T11:34:58.241052146Z
}
ignasirv commented 1 month ago

Hi I would need to know: 1- The init smtLevel at Erigon vs the smtLevel at legacy 2- The VirtualCountersSmtReduction value at erigon config 3- The batch has 362 blocks where the las block has 10 ether transfers (all other blocks are empty). Which is the vcounter 'P' consmption by erigon for each empty block and for the last block?

Sharonbc01 commented 1 month ago

Hey @ignasirv do you have an update on this issue please?

ignasirv commented 1 month ago

Hi, I need to know the info requested in my previous message to understand what could have happened

Sharonbc01 commented 1 month ago

Hey @mandrigin can you advise on this please for @ignasirv when you have a chance. Thanks so much!

Sharonbc01 commented 1 month ago

Hey @mandrigin please can you share information requested on this issue to @ignasirv Thanks.

hexoscott commented 3 weeks ago

Hi @ignasirv some questions/comments on the questions:

  1. do you mean the smt depth here? This is based on the realtime depth calc by erigon and isn't logged at the moment, I will try and find where Bali is at for you. With regards to legacy, here we mean the stateless executor, I'm not sure how that initialises the SMT depth, but Fran will know, now sure on his github handle to @ him.
  2. I have asked Rafa for this config so will feed back once we have it, but I'd guess this isn't set so will be 0.
  3. We don't track counter consumption on a per block level, just at the batch level. I can shadowfork Bali to get this but to what level do you need? Every block we add increases the batch level counters based on batchL2Data etc. should we include this or do you want to see something else?
ignasirv commented 3 weeks ago

1- Yes, I think this poseidon undercount may be because we are using a lower tree depth for erigon than for legacy/executor. The depth used by the executor is the correct depth, I guess legacy uses the same, @agnusmor can confirm 2- If we are setting the value to 0 it might be a problem there because it will set the smt depth to zero if it is not correctly handled in the golan code where puts a default value like 0.6 in case the values is 0 (undefined in js), would be worth to check 3- Okey, don't think the problem is there but wanted to confirm