RandomBeaconHistory contract #375
Conversation
contracts/SourceOfRandomness.cdc
Outdated
| // called every block in the system transaction | ||
|
|
||
| // only callable in the system transaction and by the service account | ||
| let blockEntropy = entropy() |
There was a problem hiding this comment.
So this is like getCurrentBlock() in that it doesn't require an import or anything?
What happens if a different account tries to call it? will it just throw a runtime error?
There was a problem hiding this comment.
So this is like getCurrentBlock() in that it doesn't require an import or anything?
Exactly!
What happens if a different account tries to call it? will it just throw a runtime error?
Yes. It can only be called in the system transaction actually. Otherwise it panics.
There was a problem hiding this comment.
what does entropy() function do ?
There was a problem hiding this comment.
@bluesign This is part of a proposal I'm writing a FLIP about : https://github.com/onflow/flips/pull/123/files . It's still a draft but I'm sharing an early preview of what the code changes are about. The changes will remain a draft till the FLIP is discussed and maybe approved.
There was a problem hiding this comment.
thanks @tarakby, this is a great idea (FLIP). On the implementation side, why does it have to be a function ( entropy() ) ? Why not send entropy as a parameter to heartbeat?
I worry a bit there will be a std lib function, but not callable and it can confuse users a bit.
There was a problem hiding this comment.
Why not send entropy as a parameter to heartbeat?
That was the other option here, but that causes some issues due to the reliance on the system TX always having the same tx hash. I would like to get rid of that problem: onflow/flow-go#4584 (comment) but I didnt want to hold back the development of this. We can later switch to arguments...
...hold on I just realised we can implement this in a way so that later when we switch to system tx arguments we don't need to make any changes to the contract... let me make that change.
There was a problem hiding this comment.
Made some changes. It should also be easier to test everything this way https://github.com/onflow/flow-go/pull/4582/files#diff-73e7c4fe793765e3190e62aecdfd6e14b10d5226f61d26a56551192e4df7d301R20
30f8ad4 to
eac451f
Compare
|
See #383 for my inclusion to this PR branch |
|
I will update the flow go PR to use this |
Co-authored-by: Tarak Ben Youssef <50252200+tarakby@users.noreply.github.com>
5f587de to
83d645a
Compare
83d645a to
828f80d
Compare
transactions/randomBeaconHistory/record_source_of_randomness.cdc
Outdated
Show resolved
Hide resolved
e3ab7a9 to
98cd8f3
Compare
| /// Retrieves the source of randomness for the requested block height from the RandomBeaconHistory contract. | ||
| /// | ||
| access(all) fun main(page: UInt64, perPage: UInt64): RandomBeaconHistory.RandomSourceHistoryPage { | ||
| return RandomBeaconHistory.getRandomSourceHistoryPage(page: UInt64, perPage: UInt64) |
There was a problem hiding this comment.
I am writing some tests for this contract, using the Cadence testing framework, and came across a bug in this script:
RandomBeaconHistory.getRandomSourceHistoryPage(page: page, perPage: perPage)There was a problem hiding this comment.
I am working with a custom-built flow-cli version, with upcoming changes on the Cadence testing framework.
import Test
import BlockchainHelpers
import "RandomBeaconHistory"
access(all) let admin = Test.getAccount(0x0000000000000007)
access(all)
fun setup() {
let err = Test.deployContract(
name: "RandomBeaconHistory",
path: "../contracts/RandomBeaconHistory.cdc",
arguments: []
)
Test.expect(err, Test.beNil())
}
access(all)
fun testGetLowestHeightWhenNotInitialized() {
let scriptResult = executeScript(
"../scripts/get_lowest_height.cdc",
[]
)
Test.expect(scriptResult, Test.beFailed())
Test.assertError(
scriptResult,
errorMessage: "History has not yet been initialized"
)
}
access(all)
fun testGetRandomSourceHistoryPageWithoutLowestHeightSet() {
let page: UInt64 = 1
let perPage: UInt64 = 10
let scriptResult = executeScript(
"../scripts/get_source_of_randomness_page.cdc",
[page, perPage]
)
Test.expect(scriptResult, Test.beFailed())
Test.assertError(
scriptResult,
errorMessage: "History has not yet been initialized",
)
}
access(all)
fun testGetSourceOfRandomnessWithoutLowestHeightSet() {
let atBlockHeight: UInt64 = 101
let scriptResult = executeScript(
"../scripts/get_source_of_randomness.cdc",
[atBlockHeight]
)
Test.expect(scriptResult, Test.beFailed())
Test.assertError(
scriptResult,
errorMessage: "History has not yet been initialized"
)
}
access(all)
fun testRecordRandomSource() {
let randomSource: [UInt8] = [0, 1, 1, 2, 3, 5, 8]
let txResult = executeTransaction(
"../transactions/record_random_source.cdc",
[randomSource],
admin
)
Test.expect(txResult, Test.beSucceeded())
let page: UInt64 = 0
let perPage: UInt64 = 10
let scriptResult = executeScript(
"../scripts/get_source_of_randomness_page.cdc",
[page, perPage]
)
Test.expect(scriptResult, Test.beSucceeded())
let history = (scriptResult.returnValue as! RandomBeaconHistory.RandomSourceHistoryPage?)!
Test.assertEqual(randomSource, history.values[0]!.value)
}
access(all)
fun testGetSourceOfRandomnessForMissingBlockHeight() {
let atBlockHeight: UInt64 = 101
let scriptResult = executeScript(
"../scripts/get_source_of_randomness.cdc",
[atBlockHeight]
)
Test.expect(scriptResult, Test.beFailed())
Test.assertError(
scriptResult,
errorMessage: "Source of randomness not yet recorded"
)
}
access(all)
fun testGetSourceOfRandomnessPrecedingRecordedHistory() {
let lowestHeight = (executeScript(
"../scripts/get_lowest_height.cdc",
[]
).returnValue as! UInt64?)!
let atBlockHeight: UInt64 = lowestHeight - 2
let scriptResult = executeScript(
"../scripts/get_source_of_randomness.cdc",
[atBlockHeight]
)
Test.expect(scriptResult, Test.beFailed())
Test.assertError(
scriptResult,
errorMessage: "Requested block height precedes recorded history"
)
}
access(all)
fun testGetSourceOfRandomnessFromPreviousBlockHeight() {
// Commit current block and advance to the next one
Test.commitBlock()
let atBlockHeight: UInt64 = getCurrentBlockHeight() - 1
let scriptResult = executeScript(
"../scripts/get_source_of_randomness.cdc",
[atBlockHeight]
)
Test.expect(scriptResult, Test.beSucceeded())
}
access(all)
fun testGetRandomSourceHistoryPageExceedingLastPage() {
// There is only 1 page currently
let page: UInt64 = 11
let perPage: UInt64 = 10
let scriptResult = executeScript(
"../scripts/get_source_of_randomness_page.cdc",
[page, perPage]
)
Test.expect(scriptResult, Test.beSucceeded())
let history = (scriptResult.returnValue as! RandomBeaconHistory.RandomSourceHistoryPage?)!
Test.expect(history.values, Test.beEmpty())
}When a release is made available, I'll try to test most of the core contracts using the Cadence testing framework.
There was a problem hiding this comment.
nice catch! Thank you.
And thank you for writing tests!
There was a problem hiding this comment.
Well done on the contract implementation 💯 👏
There was a problem hiding this comment.
Very nice and readable tests.
There was a problem hiding this comment.
Great work @janezpodhostnik and @m-Peter! Maybe the tests can be added to this PR or in a follow-up PR to the repo?
There was a problem hiding this comment.
@turbolent I will certainly follow up and add these tests and setup a CI for them to run. However, as of now, there is no release of flow-cli that can run this version of the testing framework. I am using a custom build.
| /// | ||
| access(all) fun sourceOfRandomness(atBlockHeight: UInt64): RandomSource { | ||
| pre { | ||
| self.lowestHeight != nil: "History has not yet been initialized" |
There was a problem hiding this comment.
Side note: It would be great to have a way of optional-binding in conditions, e.g. be able to say let lowestHeight = self.lowestHeight here instead, like in an if let, and then use lowestHeight instead of the repeated self.lowestHeight! in the condition below and in the body
|
@turbolent Please check again. |
* Minimal source of randomness contract * Add call to entropy * Source of randomness history suggestions (#377) * change sor heartbeat signature * RandomBeaconHistory impl + transactions & scripts * simplify RandomBeaconHistory interface * rm RandomBeaconHistory.HeartbeatPublicPath * cleanup RandomBeaconHistory formatting & naming * update go assets * update RandomBeaconHistory events & err messages * update go assets * Update contracts/RandomBeaconHistory.cdc Co-authored-by: Tarak Ben Youssef <50252200+tarakby@users.noreply.github.com> * update go assets * rename RandomBeaconHistory.initHeight -> .lowestHeight * add RandomBeaconHistory.getRandomSourceHistoryPage() * add RandomBeaconHistory scripts * update go assets * fix RandomBeaconHistory typo * update go assets * update RandomBeacon naming * remove unused transaction and rename * fix get_source_of_randomness script * Adress review comments * change signature of sourceOfRandomness * fix tidy --------- Co-authored-by: Tarak Ben Youssef <50252200+tarakby@users.noreply.github.com> Co-authored-by: Giovanni Sanchez <108043524+sisyphusSmiling@users.noreply.github.com>
* Minimal source of randomness contract * Add call to entropy * Source of randomness history suggestions (#377) * change sor heartbeat signature * RandomBeaconHistory impl + transactions & scripts * simplify RandomBeaconHistory interface * rm RandomBeaconHistory.HeartbeatPublicPath * cleanup RandomBeaconHistory formatting & naming * update go assets * update RandomBeaconHistory events & err messages * update go assets * Update contracts/RandomBeaconHistory.cdc Co-authored-by: Tarak Ben Youssef <50252200+tarakby@users.noreply.github.com> * update go assets * rename RandomBeaconHistory.initHeight -> .lowestHeight * add RandomBeaconHistory.getRandomSourceHistoryPage() * add RandomBeaconHistory scripts * update go assets * fix RandomBeaconHistory typo * update go assets * update RandomBeacon naming * remove unused transaction and rename * fix get_source_of_randomness script * Adress review comments * change signature of sourceOfRandomness * fix tidy --------- Co-authored-by: Tarak Ben Youssef <50252200+tarakby@users.noreply.github.com> Co-authored-by: Giovanni Sanchez <108043524+sisyphusSmiling@users.noreply.github.com>
related to: onflow/flow-go#4582
This is the contract for onflow/flips#123