-
Notifications
You must be signed in to change notification settings - Fork 468
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
Pack more bits into blobs #2101
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
accBits -= 8 | ||
} | ||
} | ||
if accBits != 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, this should never happen because 4096 is divisible by 4 and the pattern for accBits is 2, 4, 6, 0, repeat on encoding and 6, 4, 2, 0, repeat on decoding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly, though this condition (well the one on encoding) helped track down a bug before where I exited the loop early and left some bits in the accumulator
} | ||
if accBits != 0 { | ||
return nil, fmt.Errorf("somehow ended up with %v spare accBits", accBits) | ||
} | ||
} | ||
var outputData []byte | ||
err := rlp.Decode(bytes.NewReader(rlpData), &outputData) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we already have the 1000 byte buffer so I think it's fine, but do we need to take into account the RLP length prefix of the data in the max bytes encoded per blob?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that's a good point, though I think we'll just fit in the 1000
Points into #2095
In addition to utilizing all whole bytes of each blob field element, this PR also utilizes all whole bits of each field element, increasing encoding efficiency by roughly 2.5%.
This PR also includes a test of blob encoding and decoding.