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

type safety: create types for SenderRandomness, ReceiverPreimage, etc. #195

Open
dan-da opened this issue Oct 1, 2024 · 0 comments
Open

Comments

@dan-da
Copy link
Collaborator

dan-da commented Oct 1, 2024

Presently we have several logical types that all have the concrete type Digest.

For example, these are all taken from grepping the code:

  • block_digest: Digest,
  • sender_randomness: Digest,
  • receiver_preimage: Digest,
  • transaction_digest: Digest,
  • type_script_hash: Digest,
  • lock_script_hash: Digest,
  • receiver_privacy_digest: Digest,
  • receiver_privacy_preimage: Digest,
  • kernel_digest: Digest,
  • swbfa_digest: Digest,
  • utxo_digest: Digest,
  • sync_label: Digest,
  • unlock_key: Digest,
  • seed: Digest,
  • spending_lock: Digest,

...and so on.

This can get particularly confusing when tuples are used, eg:

pub fn make_item_and_randomnesses() -> (Digest, Digest, Digest) {}

let addition_record_to_utxo_info: HashMap<AdditionRecord, (Utxo, Digest, Digest)> = ...;

pub async fn find_path(&self, start: Digest, stop: Digest,) -> (Vec<Digest>, Digest, Vec<Digest>)

When a fn returns a tuple of Digest, it becomes necessary to read the fn implementation (or if lucky, docs) in order to determine what each Digest represents. Worse, the compiler does not know the logical type, and thus cannot enforce type safety beyond Digest.

The same applies for fn parameters. At least they are named, but sometimes may contain unnamed tuples, eg Vec<(Digest, ..)>.

So it would be a win for type-safety and maintainability if we were to strongly type these logical types. So then eg SenderRandomness can only be used where SenderRandomness is expected.

Unfortunately, rust does not make this a trivial task.

At first, one might think we could just use type aliases, eg:

type SenderRandomness = Digest;

This can indeed help readability, however the compiler does nothing to enforce type-safety. A SenderRandomness alias can be passed to a fn that expects a Digest and vice-versa. This is not a solution.

Thus developers typically use a NewType struct for this:

struct SenderRandomness(Digest)

This works. The compiler will enforce type-safety for the newtype. However, to be fully backwards-compatible, the newtype must re-implement the entire public interface of Digest.

There is one possible shortcut, though it is frowned upon.

The newtype can impl Deref, and possibly DerefMut.

In that case, all methods from "impl Digest" are callable on SenderRandomness.

The downside is that we lose some type safety if a variable is passed by reference, eg a fn that with an &Digest param would happily accept an &SenderRandomness because the deref() method returns an &Digest.

In the case of Digest, it is Copy and is normally passed by value anyway. So that's not much of an issue. It is a code-smell though, and is not intended usage for Deref, which exists to support smart-pointer usage. Anyway its an option.

Digest presently only has about 6 methods and 1 public field. So not impractical to impl wrapper methods.

It also has 14 impl x for y and 12 derive() impls. The former can be impl'd in the newtype with a simple one-line wrapper for each.

So then, considering all the above:

My thought is to create a macro that impls the newtype pattern for each logical type that we wish to be distinct.

Existing code can then be mechanically transformed to use the newtype instead of Digest.

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

No branches or pull requests

1 participant