Skip to content

Fix isAuthorized task by changing a test that check for empty hex#926

Merged
antho1404 merged 2 commits intodevfrom
ss/marketplace-fix-isAuthorized
May 1, 2019
Merged

Fix isAuthorized task by changing a test that check for empty hex#926
antho1404 merged 2 commits intodevfrom
ss/marketplace-fix-isAuthorized

Conversation

@NicolasMahe
Copy link
Copy Markdown
Member

The function hexToString needs a valid utf8 to not throw an error.
It was used to test if the sidHash was empty or not. The sidHash is not an utf8 but a keccak.
I replaced it by a conversion to number to make the same test.

// get service from version hash
const sidHash = await contract.methods.versionHashToService(hashToHex(versionHash)).call()
if (hexToString(sidHash) === "") {
if (Number(sidHash) === 0) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what type sidHash is? what about Number overflow? are you 100% sure that there is not a signel sidHash that give 0 during the cast?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried with multiple values and it worked correctly:

> Number('0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff') === 0 ? "nok" : "ok"
'ok'
> Number('0x0000000000000000000000000000000000000000000000000000000000000001') === 0 ? "nok" : "ok"
'ok'
> Number('0x164475976c7f1b9abeee539b6fc35097c31148be1957c0746f30069b4ad022a4') === 0 ? "nok" : "ok"
'ok'
> Number('0x0000000000000000000000000000000000000000000000000000000000000000') === 0 ? "nok" : "ok"
'nok'

The function versionHashToService returns 0x0000000000000000000000000000000000000000000000000000000000000000 when the versionHash is not found.

@antho1404 antho1404 merged commit 9f95e20 into dev May 1, 2019
@antho1404 antho1404 deleted the ss/marketplace-fix-isAuthorized branch May 1, 2019 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants