Skip to content
This repository has been archived by the owner on Nov 18, 2021. It is now read-only.

Add unit test for genesis file creation #56

Closed
wants to merge 2 commits into from

Conversation

n-hutton
Copy link
Contributor

@n-hutton n-hutton commented Nov 8, 2019

No description provided.

pytest {posargs}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Allows you to run a single test tox -e py37 -- tests/genesis/test_genesis.py

DESIRED_ENTITIES = 30
DESIRED_MINERS = 20
TOKENS_PER_ENTITY = TOTAL_TOKEN_SUPPLY/DESIRED_ENTITIES
TOKENS_STAKED_PER_MINER = TOKENS_PER_ENTITY/2
TOKENS_PER_ENTITY = int(TOTAL_TOKEN_SUPPLY/DESIRED_ENTITIES)
Copy link

Choose a reason for hiding this comment

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

Could we need these to be long instead of int to support any token supply?

Copy link
Contributor

Choose a reason for hiding this comment

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

Python3+ doesn't have a long type, the int type supports any length of integer in pure python - we only need to be careful of overflows if using libraries (e.g.: numpy/pandas): https://mortada.net/can-integer-operations-overflow-in-python.html

DESIRED_ENTITIES = 30
DESIRED_MINERS = 20
TOKENS_PER_ENTITY = TOTAL_TOKEN_SUPPLY/DESIRED_ENTITIES
TOKENS_STAKED_PER_MINER = TOKENS_PER_ENTITY/2
TOKENS_PER_ENTITY = int(TOTAL_TOKEN_SUPPLY/DESIRED_ENTITIES)
Copy link
Contributor

Choose a reason for hiding this comment

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

Python3+ doesn't have a long type, the int type supports any length of integer in pure python - we only need to be careful of overflows if using libraries (e.g.: numpy/pandas): https://mortada.net/can-integer-operations-overflow-in-python.html

TOKENS_PER_ENTITY = TOTAL_TOKEN_SUPPLY/DESIRED_ENTITIES
TOKENS_STAKED_PER_MINER = TOKENS_PER_ENTITY/2
TOKENS_PER_ENTITY = int(TOTAL_TOKEN_SUPPLY/DESIRED_ENTITIES)
TOKENS_STAKED_PER_MINER = int(TOKENS_PER_ENTITY/2)
Copy link
Contributor

Choose a reason for hiding this comment

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

You could use floor division here instead of casting to int, this will produce the same result so long as you're only dealing with positive numbers ( 5 // 2 == 2 == int(5/2), -5 // 2 == -3, int(-5/2) == -2)

Suggested change
TOKENS_STAKED_PER_MINER = int(TOKENS_PER_ENTITY/2)
TOKENS_STAKED_PER_MINER = TOKENS_PER_ENTITY // 2

TOTAL_TOKEN_SUPPLY = 1000000
TOKENS_ALLOCATED_FOR_MINERS = 100
TOKENS_STAKED_PER_MINER = int(TOKENS_ALLOCATED_FOR_MINERS / DESIRED_MINERS)
TOKENS_PER_ENTITY = int((TOTAL_TOKEN_SUPPLY-(TOKENS_ALLOCATED_FOR_MINERS)) /DESIRED_ENTITIES)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
TOKENS_PER_ENTITY = int((TOTAL_TOKEN_SUPPLY-(TOKENS_ALLOCATED_FOR_MINERS)) /DESIRED_ENTITIES)
TOKENS_PER_ENTITY = int((TOTAL_TOKEN_SUPPLY - TOKENS_ALLOCATED_FOR_MINERS) / DESIRED_ENTITIES)

@@ -103,6 +103,9 @@ def as_json_object(self):
for (entity, amount, stake_amount) in self._entities_with_wealth:
token_address = generate_token_address(entity.public_key)

if not isinstance(amount, int) or not isinstance(stake_amount, int):
raise TypeError(f"Initial token amount and stake amount must be integer values! Amount: {amount}, Stake {stake_amount}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Format strings aren't supported in python 3.5.

Suggested change
raise TypeError(f"Initial token amount and stake amount must be integer values! Amount: {amount}, Stake {stake_amount}")
raise TypeError("Initial token amount and stake amount must be integer values! Amount: {}, Stake {}".format(amount, stake_amount))

stake_all_accounts += account['stake']

# Check that there are as many tokens as originally planned
self.assertEqual(funds_all_accounts + stake_all_accounts, TOTAL_TOKEN_SUPPLY)
Copy link
Contributor

Choose a reason for hiding this comment

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

This will only hold if the number of tokens above is cleanly divisible, can we make this more rigorous, or otherwise handle less cleanly divisible supplies?

@ejfitzgerald
Copy link
Member

Think that we might remove the Genesis file from the API so going to close this PR

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants