feat: create compact share counter #1657
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1657 +/- ##
==========================================
+ Coverage 51.11% 51.56% +0.44%
==========================================
Files 94 95 +1
Lines 5894 5954 +60
==========================================
+ Hits 3013 3070 +57
- Misses 2569 2570 +1
- Partials 312 314 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| tailShares, err := TailPaddingShares(wantShareCount - currentShareCount) | ||
| if err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
[no change needed] In the past we've observed edge cases where wantShareCount < currentShareCount which will now cause a panic here. Given this code will likely go away soon w/ deterministic square layout I think it's worth just being mindful of it in case it comes up in tests / fuzzing
Co-authored-by: Rootul P <rootulp@gmail.com>
Co-authored-by: Rootul P <rootulp@gmail.com>
| {txs: []coretypes.Tx{newTx(120)}}, | ||
| {txs: []coretypes.Tx{newTx(appconsts.FirstCompactShareContentSize - 2)}}, | ||
| {txs: []coretypes.Tx{newTx(appconsts.FirstCompactShareContentSize - 1)}}, | ||
| {txs: []coretypes.Tx{newTx(appconsts.FirstCompactShareContentSize)}}, | ||
| {txs: []coretypes.Tx{newTx(appconsts.FirstCompactShareContentSize + 1)}}, | ||
| {txs: []coretypes.Tx{newTx(appconsts.FirstCompactShareContentSize), newTx(appconsts.ContinuationCompactShareContentSize - 4)}}, | ||
| {txs: newTxs(1000, 100)}, | ||
| {txs: newTxs(100, 1000)}, |
There was a problem hiding this comment.
[no change needed] the inputs to this test could also be fuzzed given we just need a random dataLen
There was a problem hiding this comment.
Yeah I can explore that in a follow up. I think these cover the edge cases that come to mind
As part of ADR 020, we need something to iteratively calculate how many shares both pfbs and txs would take up should they be added. This PR creates a
CompactShareCounterto perform this job. It would have been also possible to extend theCompactShareSplitterto do this task but we would have needed to add the functionality to "undo" transactions (because they were too large) and this seemed like quite some functionality to add.Ref: #1214
Checklist