Conversation
bd137ae to
ebb60f9
Compare
Batch times mean how frequently we ping the mempool to give us a proposed batch, right? There are not indications of how long it'll take to prove them. So, I think lowering to 1s is fine, and we could potentially lower it even more. I do expect batches to take some time to prove and maybe initially closer to 5 seconds. |
Correct, though this is additionally constrained by the number of batch "workers". So if there is no worker available at the next tick, then no batch is created. This logic should probably be revisited closer to production so that batches are created as fast as possible while there are still transactions available. But we can cross that bridge once proving etc or capacity becomes an issue so we have a clearer picture of this pipeline should look like imo. |
|
@igamigo brought up a good point (#1437 (comment)) which is that this also reduces the absolute time the client has for querying account history unless we also increase the depth there. cc @drahnr ^^ do you know if the various proof history was chosen with an absolute time in mind, or was it more a spitball of |
|
I think it was chosen somewhat arbitrarily as "not too big and not too small". I do think it should be a value that is somewhere between 1 and 10 minutes, and with block times going to 2 seconds, the current setting of 30 is right at ~1 minute. So, maybe we should bump it up to 60 - or maybe even 100. Though, querying things later in history would becomes somewhat more expensive (I don't remember what would be the absolute difference in extra latency between 30 and 100). |
I've bumped it to |
igamigo
left a comment
There was a problem hiding this comment.
Not sure about the performance consequences of tripling the account tree history but it sounds like it needs benchmarking anyway
|
33 was somewhere between 10 and 100, no further thought was put into it. It had a small impact on design, so bumping it makes adding some caching logic to |
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! I've decided to make some last-minute changes to the parameters :)
I've also lowered the batch interval from
2s -> 1swhich at least gives us time for multiple batches per block. There is an argument that it should be even lower?Closes #1437