Set default Delta value to 3 seconds #191
Conversation
ranchalp
left a comment
ranchalp
left a comment
There was a problem hiding this comment.
Looks good except for the comment: my expectation is that 3s is the propagation time, and gpbft uses the RTT 2*delta.
Update the default delta value to 3 seconds and document the rationale behind it. The defaults may change in the future once concrete measurements on a live network are performed.
masih
force-pushed
the
masih/defalut-delta-3s
branch
from
3601315 to
03f0a0b
Compare
Comment adjusted; thank you |
jsoares
left a comment
jsoares
left a comment
There was a problem hiding this comment.
See comment on comment, looks good otherwise.
| // The default of 3 seconds for the value of Dela is based on the current EC | ||
| // propagation time (i.e. round-trip time of 6 seconds) as the initial | ||
| // propagation timeout. The actual propagation time may be significantly lower. |
There was a problem hiding this comment.
EC doesn't assume a 6s round-trip time, as that's not even well-defined for a pubsub network. What it assumes is an upper bound on the gossipsub network-wide propagation time, which is actually 10s now, following filecoin-project/lotus#9290.
The increase from 6s to 10s was due to having time to spare in the epoch envelope due to faster proof computation; it does not imply the need to go higher than 6s. The value remains user-configurable, 6s worked well, and we have reason to believe that we can go lower.
I think we can even skip talking about EC and just say that we have reason to believe, based on previous data, that 3s is enough.
There was a problem hiding this comment.
Noted thank you; merge queue got the PR merged. I have take note to update the comments.
There was a problem hiding this comment.
Oops, forgot we had it enabled in the repo 🤦
There was a problem hiding this comment.
It's my fault. I added this pr to queue before your excellent review.
Fix the default delta value documentation to address previous review comment; see: * #191 (comment)
Fix the default delta value documentation to address previous review comment; see: * #191 (comment)
Fix the default delta value documentation to address previous review comment; see: * filecoin-project/go-f3#191 (comment)
Fix the default delta value documentation to address previous review comment; see: * filecoin-project/go-f3#191 (comment)
Fix the default delta value documentation to address previous review comment; see: * filecoin-project/go-f3#191 (comment)
Fix the default delta value documentation to address previous review comment; see: * filecoin-project/go-f3#191 (comment)
Fix the default delta value documentation to address previous review comment; see: * filecoin-project/go-f3#191 (comment)
Fix the default delta value documentation to address previous review comment; see: * filecoin-project/go-f3#191 (comment)
Fix the default delta value documentation to address previous review comment; see: * filecoin-project/go-f3#191 (comment)
Update the default delta value to 3 seconds and document the rationale behind it.
The defaults may change in the future once concrete measurements on a live network are performed.