[Cadence] FLIP - random function

[tarakby]

Proposes a FLIP to:

• Update the unsafeRandom function name after the underlying implementation has been secured using Flow protocol native random beacon.
• Update the interface to a safer and more convenient one (generalized types and a modulo parameter)

[dsainati1]

My only concern with this change is that I think it may communicate the wrong thing to Cadence users. I understand how from the protocol perspective our randomness is no longer "unsafe", but from the perspective of a Cadence user, I think they will probably not consider it to be such. As far as they are concerned, the randomness (used naively) is still "unsafe" because it is still exploitable by a transaction that reverts to postselect a favorable result. Changing the name of the function to remove the unsafe prefix I think would suggest to people that the function is "safe" in the sense that it is immune to this exploit, which it's not.

Users can use certain patterns to make their randomness truly safe, but they need to structure their code in a specific way for this to work, and I am worried that people won't realize they need to do this without the built-in warning that a name like unsafeRandom provides. IMO we should have the name (or the API) here indicate that the user needs to do something additional beyond just calling the function to get a random number; something like revertibleRandom, for example.

[tarakby]

Thanks @dsainati1

I see your point but I'll add counter-arguments to keep the discussion going:

• Although any transaction with a random call can be reverted, some applications require randomness and have no incentive in reverting the result. Flow-core contracts for instance use randomness without reverting risks. A well-respected dapp also (assigning random NFTs for example) wouldn't want to add a reverting logic to their contract. For these applications, the new exposed randomness is totally safe. Adding unsafe would even mean there is something wrong with the contract while there isn't.
• result abortion is an inherent property of atomic smart-contract platforms, rather than specific functions like randomness. The non-safety comes from the language property rather than the randomness itself. We can argue that any logic written with Cadence is revertible, and yet we do not add the prefix unsafe or revertible for all Cadence functions. Any developer should be aware that a state change in a transaction is revertible, not only when it uses randomness (a deterministic game transaction can abort if the player isn't happy with the result for instance)
• a language should try to provide as safe as possible tools, but it can't guarantee that any program written in that language is 100% safe. There is always some responsibility that falls on the language developer. I guess it's possible to write very unsafe contracts and Cadence can't prevent that (I'm thinking of the cryptography tools as an example).

[turbolent]

Great points @dsainati1 and @tarakby! @tarakby Could you please incorporate the concerns and your feedback into the FLIP itself? That way the reasoning is not lost

[turbolent]

Looks good, though it would be nice to provide a more convenient API (a combination of the alternatives?) which reduces potential misuse

[joshuahannan]

Looks good! I think I'm fine with accepting that developers need to know that random is revertible without reading the function name

[janezpodhostnik]

Would it be better if FVM was injecting the random function (via Environment.Declare(valueDeclaration stdlib.StandardLibraryValue)) so that it would be entirely handled by the FVM this way we could remove the random from the runtime interface entirely. @turbolent?

[turbolent]

@janezpodhostnik That could be an option. There's always the question what should be part of the built-in functionality / standard library, and what is only provided by a particular environment.

So far, the random function is part of the built-in functionality (standard library). I feel like every implementation of Cadence should provide one, so developers can be assured the function is available and reason about the behaviour. For now I'd keep it that way, however, I don't feel strongly about it.

(The same question / reasoning applies to other functions/APIs (like the block API).

[turbolent]

Great improvements! Just some fixes for the examples, and some stylistic recommendations

[AlexHentschel]

thank you for this excellent write-up 👏

[turbolent]

👍

[turbolent]

This is a great explanation:ok_hand:

Once merged, we should put this into the documentation and an abridged version into the docstring.

[turbolent]

The plan is to decide on this FLIP in the next Cadence Language Design Meeting on Tue 12th, Sep.

So far there is positive sentiment. Unless there are any new insights or significant reasons to reject are brought forward, the plan is to approve the FLIP.

[turbolent]

@tarakby The FLIP got approved 🎉 Please update the status and merge :-)