flow-go icon indicating copy to clipboard operation
flow-go copied to clipboard

[EVM] Revertible random Cadence arch function

Open devbugging opened this issue 1 year ago • 1 comments

Closes: #5862

This PR adds a function to the Cadence arch that can be used to produce revertible random.

The function can be called from Solidity like so:

cadenceArch.staticcall(abi.encodeWithSignature("revertibleRandom()"))

and it returns a random uint64 value.

devbugging avatar May 16 '24 14:05 devbugging

Codecov Report

Attention: Patch coverage is 54.54545% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 55.61%. Comparing base (4f85bc2) to head (5b63d8e).

Files Patch % Lines
fvm/evm/handler/precompiles.go 25.00% 5 Missing and 1 partial :warning:
fvm/evm/precompiles/arch.go 71.42% 2 Missing and 2 partials :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5934      +/-   ##
==========================================
- Coverage   55.61%   55.61%   -0.01%     
==========================================
  Files        1128     1128              
  Lines       88951    88973      +22     
==========================================
+ Hits        49470    49482      +12     
- Misses      34776    34781       +5     
- Partials     4705     4710       +5     
Flag Coverage Δ
unittests 55.61% <54.54%> (-0.01%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar May 16 '24 14:05 codecov-commenter

how about the random source exposed in the EVM block field? are we planning to remove/update it as part of this PR too? I can't remember how that source is derived.

@tarakby that random value is derived in the same manner by using the Go implemented PRG using the same source. I believe that is ok, and the value being in the block field is something I would keep because some dapps rely on it. If you want to take a look, this is where the value is set: https://github.com/onflow/flow-go/blob/eda4a174ae9141343fa7989eaceace22512be5e6/fvm/evm/handler/handler.go#L422

The Cadence signature of revertible Random is different

Yes, I'm aware, beside Solidity function not having the flexibility Cadence has, it's not an issue otherwise.

devbugging avatar May 17 '24 12:05 devbugging

https://github.com/onflow/flow-go/blob/eda4a174ae9141343fa7989eaceace22512be5e6/fvm/evm/handler/handler.go#L422

In the implementation of revertibleRandom, the backend has a PRG initialized using the tx hash as a salt (of the Flow tx where the EVM call happened). In the code you mentioned, where the EVM block field gets filled, how would the PRG be initialized? I mean what tx would be taken into account as a salt? I am assuming an EVM block is formed by many txs so how do we pick one as a salt? Or is there one EVM block for each Flow tx? 🤔

tagging @sideninja and @ramtinms

tarakby avatar May 20 '24 17:05 tarakby

@tarakby that's a good callout. So you are right, a single Flow transaction can contain multiple EVM transactions / blocks. So each EVM block will use the Flow transaction as a seed to the PRG, if my understanding is correct. I believe that each subsequent call of the ReadRandom should give the next PRG value using still the same Flow transaction as a seed. Is that problematic? Also this is not really part of the above PR, it was added before but I don't mind where we discuss it.

devbugging avatar May 31 '24 16:05 devbugging

Ok, so my understanding (from your answer and looking at the code) is that each Flow transaction has a backend, which initializes a new fresh PRG using the Flow transaction ID as a seed. That PRG will be used for all EVM revertibleRandom calls related to EVM within that Flow transaction. This is good and is indeed the intended behaviour, as long as the PRG isn't re-initialized within that same Flow transaction.

Now what my question is about, is not the revertibleRandom calls, but the EVM block random field, and what PRG/salt it uses. From your answer, I understand that a Flow transaction can contain many EVM blocks, and that the random field of each block is filled by the same unique PRG of the Flow transaction. This is also the intended behaviour, so all good here too unless you think I made a mistake.

Also this is not really part of the above PR

Right, the code wasn't added in this PR, but this PR closes https://github.com/onflow/flow-go/issues/5862 which is supposed to make sure the EVM block field doesn't expose any value that helps predicting the revertibleRandom outputs. This is why I was following up here. The implementation looks good to me.

tarakby avatar May 31 '24 18:05 tarakby

Ok, so my understanding (from your answer and looking at the code) is that each Flow transaction has a backend, which initializes a new fresh PRG using the Flow transaction ID as a seed. That PRG will be used for all EVM revertibleRandom calls related to EVM within that Flow transaction. This is good and is indeed the intended behaviour, as long as the PRG isn't re-initialized within that same Flow transaction.

Now what my question is about, is not the revertibleRandom calls, but the EVM block random field, and what PRG/salt it uses. From your answer, I understand that a Flow transaction can contain many EVM blocks, and that the random field of each block is filled by the same unique PRG of the Flow transaction. This is also the intended behaviour, so all good here too unless you think I made a mistake.

Also this is not really part of the above PR

Right, the code wasn't added in this PR, but this PR closes #5862 which is supposed to make sure the EVM block field doesn't expose any value that helps predicting the revertibleRandom outputs. This is why I was following up here. The implementation looks good to me.

I very much appreciate you asking these questions, I could make a mistake. But the way it's currently made, is that each EVM block random field when inside a single Flow transaction will generate the next pseudo-random value seeded by the tx ID. This should not to my understanding expose anything that would predict the value in the reversible random.

devbugging avatar Jun 03 '24 10:06 devbugging