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

Truncation of SlotId values in the persistence layer. #1048

Closed
jonathanknowles opened this issue Nov 19, 2019 · 2 comments
Closed

Truncation of SlotId values in the persistence layer. #1048

jonathanknowles opened this issue Nov 19, 2019 · 2 comments

Comments

@jonathanknowles
Copy link
Member

jonathanknowles commented Nov 19, 2019

Release Operating System Cause
next All Code

Context

SlotId is a 96-bit wide unsigned integer type, consisting of:

Values of the SlotId type, when stored in the database, are represented by 64-bit wide signed values, which in practice means that we have 63 bits available for each value (since we coerce the unsigned value to a signed value).

This means that the most significant 33 bits of an EpochNo are not usable, as they must be truncated away when persisting a SlotId to the database.

Why our current solution is bad

  1. We're able to represent values in the wallet layer that cannot be safely persisted in the DB layer. Our choice of types implies that we support EpochNo values up to 64 bits in length, but our DB layer only supports EpochNo values that are up to 31 bits in length.

  2. We have an unenforced invariant in the system: EpochNo values should not be larger than 2^32 − 1. (Remember that we use a Word64 to represent such values.)

  3. The means by which we truncate values, the flatSlot function, is broken for some inputs. In our test suite, we assert that the fromFlatSlot . flatSlot == id property holds, but in fact it does not. Here's a counterexample:

    > let maxEpochNo = EpochNo maxBound
    > let maxEpochLength = EpochLength maxBound
    > fromFlatSlot maxEpochLength $ flatSlot maxEpochLength (SlotId maxEpochNo (SlotNo 0))
    SlotId {epochNumber = EpochNo {unEpochNo = 4294967296}, slotNumber = SlotNo {unSlotNo = 1}}

    This counterexample is currently not discovered by our property tests, as we only test with a limited range of Slotid values.

Counterargument

Of course, it could be argued that we'll never run into issues with this truncation mechanism, as even for extremely short epochs (lasting only a few seconds), allocating only 31 bits for each EpochNo value should be enough to last us for hundreds of years.

However, if that argument holds, then why should EpochNo be a 64-bit wide value?

Solutions

There are two obvious solutions:

Solution 1: Widen the database type

Widen the DB field so that it's capable of storing all 96 bits. The simplest solution would be to use a pair of 64-bit fields, but alternatively we could use a single fixed-length unsigned hexadecimal string field (as these are comparable).

Solution 2: Narrow the primitive types

Narrow the EpochNo type so that it's capable of storing only 31 bits. When decoding EpochNo values from the outside world, fail fast if the supplied value is greater than 31 bits in length.

Additional tests we should have

  1. It should be possible to persist any arbitrary value of SlotId to the database and read it back successfully.
@KtorZ
Copy link
Member

KtorZ commented Nov 19, 2019

Let's move this to the laboratory and keep #1025 to keep track of the issue. (cc @Anviking).

@jonathanknowles
Copy link
Member Author

Discussion has moved to this issue: https://github.com/input-output-hk/cardano-wallet-laboratory/issues/16

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

No branches or pull requests

2 participants