Concern with overflow and the use of signed integers regarding the Lyra2.c file
Closed this issue · 8 comments
Your code is on the right, the Lyra2.c original code is on the left, they use a uint64_t, you use an int64_t (which as far as I know is a signed integer and can lead to overflow errors)
Not a terribly difficult fix, but it is a concern.
Cheers! Thanks for all your work, I am but a lowly Garlic miner, willing to help where I can
There is no concern, if you read the call stack you will see it can't get an invalid int. If int64_t was always a concern it wouldn't have been added.
is relying on the call stack beneficial for efficiency? I'm still just a student so I'm trying to gauge why you wouldn't just change it to a unsigned int, does it make more sense to restrict the functions parameters instead of relying on the call stack to return a legitimate value? (again im a student so bear with me if what i'm saying makes no sense)
Honestly, I don't know, it's not my code- it's from the reference lyra2 implementation. You'd have to ask them why they chose this.
the code I diffed is from the lyra2.c core implementation, so maybe there was some change between the docs? I know its annoying but can you shoot me that link to the ref docs before we call this issue closed?
I'll find tomorrow, it's later here.
https://github.com/vertcoin/vertcoin-old/blob/master/src/Lyra2RE/Lyra2.c#L46
Quick reference to the vertcoin Lyra2, for comparison
Well that's vertcoins implementation not the reference implementation
This is a none issue. The function you are referring can't be passed anything other than 32 bytes look here and you will see why https://github.com/GarlicoinOrg/Garlicoin/blob/master/src/crypto/allium/allium.c#L59.