Feat/new types #296
Feat/new types #296
Conversation
| export const SUPPORTED_FUNCTIONS: SupportedFunctions = { | ||
| add: { | ||
| supportedBits: SUPPORTED_BITS, | ||
| safeMin: true, |
There was a problem hiding this comment.
not sure to understand why you would need the safeMin to be true here, if I understand this would mean that you force the output to be of type the lowest bitwidth of inputs, but this is not needed here, because you can indeed sum a euint8 with a euint64 and result would be a euint64
There was a problem hiding this comment.
You're right, but the codegen expects a uint8 result, otherwise it fails. I think this is a mistake in the testgen.ts but I don't want to touch it now.
codegen/generateOverloads.ts
Outdated
| encryptedTests.push(safeEval(test.evalTest, smallest - 4, smallest, lhs, rhs, test.safeMin)); | ||
| } | ||
| encryptedTests.push(safeEval(test.evalTest, smallest, smallest, lhs, rhs, test.safeMin)); | ||
| encryptedTests.push(safeEval(test.evalTest, smallest, smallest - 4, lhs, rhs, test.safeMin)); |
There was a problem hiding this comment.
just noticed you were using safeEval for every test you pushed, but shouldnt we also test the wrapping behavior works correctly when overflowing? so also NOT using safeEval sometimes would make sense
There was a problem hiding this comment.
Overflowing check is already done on tfhe-rs lib, I don't think we need to go into this type of details.
There was a problem hiding this comment.
Ok, this is not so important indeed, but sometimes in rare cases we actually need to use the wrapping behavior , like what is done in the Uniswap V2 contract for the price oracle (only example I know of to be honest where overflow is desired) https://github.com/Uniswap/v2-core/blob/master/contracts/UniswapV2Pair.sol#L76
There was a problem hiding this comment.
Let's test in the contract in this case.
immortal-tofu
force-pushed
the
feat/new-types
branch
from
9e54915
to
9212a32
Compare
eboolfrom fhEVM instead of euint8euint4type and operationschore: