Skip to content
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

[audit] #2: Typos, comments, unused imports #49

Merged
merged 7 commits into from
Jul 9, 2024
Merged

Conversation

stevieraykatz
Copy link
Collaborator

From Spearbit:

Description / Recommendation

Constants.sol#L4: it should be The node hash of "eth". The . before eth is extra

Constants.sol#L8: it should be The node hash of "reverse". The . before reverse is extra

Simple Python program to check the correctness of node hashes:

from web3 import Web3
from hexbytes import HexBytes

ROOT_NODE = HexBytes(b'\0' * 32)

def sha3(s):
    return Web3.keccak(s)

def namehash(name: str) -> bytes:
    if name == '':
        return ROOT_NODE
    else:
        label, _, remainder = name.partition('.')
        return sha3(namehash(remainder) + sha3(label.encode('utf-8')))

if __name__ == '__main__':
    s: str = input()
    print(namehash(s).hex())

Constants.sol#L15: It would be best to represent BASE_ETH_NAME as hex"04_62617365_03_657468_00" with the underscores as the dnsName encoding follows the following pattern:

          w0  .               w1  . ... .               wN    ->

len(w0) | hex(w0) | len(w1) | hex(w1) | ... | len(wN) | hex(wN) | 00

and dnsEncodeName only works for words of the form w0 . w1.

EDAPrice.sol#L4, ExponentialPremiumPriceOracle.sol#L8: The following imports should be removed:

import {Test, console} from "forge-std/Test.sol";

ExponentialPremiumPriceOracle.sol#L13: PRECISION is unused.

BaseRegistrar.sol#L369: misleading comment about taking grace period into consideration should be removed

DeployRegistrarController.s.sol#L16: typo, it should be DeployRegistrarController

DeployTestnetDiscountValidator.s.sol#L7: typo, it should be DeployTestnetDiscountValidator

@stevieraykatz stevieraykatz merged commit 424792e into main Jul 9, 2024
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant