Misc consistency fixes - Githubissues

gkaracha commented 3 years ago

I thought it might be a good time to fix some small (somewhat bikesheddy) inconsistencies I'd noticed. The most important of them (and API-changing) is the change of the order of arguments for withdraw_tez: all other burrow-accessing entrypoints take the burrow id (nat) first, so for consistency withdraw_tez should too.

The other changes are of similar nature, but internal:

Rename kit_of_mukit to kit_of_denomination since the scaling factor might vary between checker instances. We already use the same name convention for liquidity.
~~Rename checkerEntrypoints.ml to checkerLazyEntrypoints.ml since it only contains the lazy entrypoints (the rest are defined in checkerMain.ml).~~ Edit: I reverted this one, so that our plots and statistics don't change.
Minor updates to README.md.
Add that .rb extension to the generate-entrypoints script; the only one without a file extension.

~~It's probably easier to review this PR per-commit.~~ Edit: After rebasing and reverting, this is probably not the case anymore :slightly_smiling_face:

github-actions[bot] commented 3 years ago

Hidden stat diff here.

github-actions[bot] commented 3 years ago

Gas costs	a2b3077e72b36013ac73f9482f5f707d13bb64c3	1d527f260f30fb18473f5ae4b03c539aaa1a4873	Diff
withdraw_tez	57584	57567	-17

Entrypoint sizes	a2b3077e72b36013ac73f9482f5f707d13bb64c3	1d527f260f30fb18473f5ae4b03c539aaa1a4873	Diff
withdraw_tez	1378	1376	-2

Test coverage: No change.

github-actions[bot] commented 3 years ago

Gas costs	6a435938ea9faa331fff4645899c9e2ef9f2e92d	275a7cf74130e015d9f6519368bdf97a1cff4db8	Diff
withdraw_tez	57584	57567	-17

Entrypoint sizes	6a435938ea9faa331fff4645899c9e2ef9f2e92d	275a7cf74130e015d9f6519368bdf97a1cff4db8	Diff
withdraw_tez	1378	1376	-2

Test coverage: No change.

gkaracha commented 3 years ago

Upon closer inspection, there are several inconsistencies around denominations.

In the docs we still refer to mulqt and mukit, going against the assumption that the scaling factor for the tokens can vary between checker instances.
For liquidity the conversion functions are named of_denomination/to_denomination which is generic, but for kit we still use of_mukit/to_mukit, which assumes a scaling factor of 1000000 (the code however opaquely refers to kit_scaling_factor_int as it should).

There are probably other naming/documentation inconsistencies around the denominations, so I'd better do that in a separate PR. For now I've moved all the non-controversial changes from this PR to #225, so I'll close this.

tezos-checker / checker

Misc consistency fixes #222