-
Notifications
You must be signed in to change notification settings - Fork 46
Conversation
@@ -0,0 +1,117 @@ | |||
//SPDX-License-Identifier: Unlicense | |||
pragma solidity >=0.7.0 <0.9.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.
Checking if this was the original version range (VLQ.sol too)? Was the code audited at a particular compiler version? We should ensure the project is configured to compile this lib with it (like the BLS lib PR comment).
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.
I've been just copying that version range, I think it originates with BLSExpander.sol
.
Was the code audited at a particular compiler version?
Nah this is new code.
We should ensure the project is configured to compile this lib with it (like the BLS lib PR comment).
I'm not sure what you mean by this. I believe that hardhat will compile it with whatever latest matching solidity version is available in the project, which is currently 0.8.15 (hardhat.config.ts has 0.6.12, 0.7.6, 0.8.15, so 0.8.15 is the latest matching version).
|
||
let exponent = 0; | ||
|
||
while (x.mod(10).eq(0) && exponent < 30) { |
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.
Why does this stop at 30? (2^256 = 10^77-ish) Is it from their code?
EDIT: I noticed that an example show's 5 bits being used for the exponent (2^5 = 32), so it looks to be by design. Most whole ethers and gas values are 10^9 to begin, so another 21 on top of that is a lot of headroom for values being compressed.
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 it's to make sure the exponent fits into a small space.
If you go above that range, you can still encode it using the VLQ part:
console.log(encodePseudoFloat(ethers.constants.MaxUint256));
// 0x0f81ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f
const [value, stream] = await pseudoFloat.decodePublic("0x55b4d7c27d"); | ||
expect(ethers.utils.formatEther(value)).to.eq("0.883887085"); | ||
expect(stream).to.eq("0x"); | ||
}); |
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.
Could test the top end of the exponent (10^30
, 10^31
).
And also 10^77
, the most significant digit 1 the rest 0 (base 10).
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.
Good idea, will followup 👍.
What is this PR doing?
Adds pseudo-floats and uses them for ethValue and gas, to reduce the number of bytes used.
By optimizing for decimal values, this format achieves a size of just 2 bytes as long as you only use 3 significant figures.
The cool thing is, the format also supports all uint256 values, which means it can be used for the fallback case. If you need 5 significant figures, pseudo-float has still got you covered with just 3 bytes.
I believe this supersedes the fixed 3-byte format proposed here, which was going to deliver 6 significant figures (only one extra significant figure for 3 bytes, at the cost of flexibility and extra bytes for value=zero and 3 sig figs).
PseudoFloats are almost twice as good as RLP for non-zero values, based on real data. (Zeros not measured, since both formats simply use
0x00
for zero.)How can these changes be manually tested?
Use
encodePseudoFloat
to encode a value in js andpseudoFloat.decodePublic
to decode it using the evm. SeepseudoFloat-test.ts
.Does this PR resolve or contribute to any issues?
Resolves #521.
Checklist
Guidelines
resolve conversation
button is for reviewers, not authors