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.

Sign up for GitHub

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

Jump to bottom

[core,model] Add ext_random_seed runtime call bindings #54

Merged
merged 14 commits into from
Apr 17, 2019
Merged

[core,model] Add ext_random_seed runtime call bindings #54

merged 14 commits into from
Apr 17, 2019

Conversation

shawntabrizi
Copy link
Contributor

@shawntabrizi shawntabrizi commented Apr 15, 2019 •
edited
Loading

Adding the ability for contracts to get the random seed of the block.

Returning the value as a Vec, although within substrate it is treated as a Hash... should/how do I use the Hash type?

Fixes #53

@shawntabrizi
[core] Add ext_random_seed runtime call bindings
e6d8236 
Adding the ability for contracts to get the random seed of the block.

Returning the value as a Vec<u8>, although within substrate it is treated as a `Hash`... should/how do I use the `Hash` type?
@Robbepop
Copy link
Collaborator

I see that fn random_seed in Substrate is returning something like <Block as BlockT>::Hash.
So this is pretty similar in respect to how we handle Address and Balance currently.
The best for pDSL users is to not have returned a Vec<u8> but instead getting the same type (<Block as BlockT>::Hash) returned. We can achieve this by doing something like this: #42 (comment)

So I think we should really first base pDSL upon node-template (happy about your ideas around that) and then we can continue integrating those calls like random_seed with proper return types.

Robbepop
Robbepop requested changes Apr 15, 2019
View reviewed changes
core/src/env/api.rs Outdated Show resolved Hide resolved
core/src/env/srml/srml_only/impls.rs Outdated Show resolved Hide resolved
core/src/env/test_env.rs Outdated Show resolved Hide resolved
core/src/env/test_env.rs Outdated Show resolved Hide resolved
core/src/env/test_env.rs Outdated
@@ -190,6 +198,11 @@ impl TestEnvData {
pub fn set_input(&mut self, input_bytes: &[u8]) {
self.input = input_bytes.to_vec();
}

/// Sets the random seed for the next contract invocation.
pub fn set_random_seed(&mut self, random_seed_bytes: &[u8]) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess in Substrate we are missing a way to mirror setting a random seed, right?
Not sure about this interface but it should be fine for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only added it cause I have no idea how the random seed would be generated otherwise...

core/src/env/test_env.rs Outdated Show resolved Hide resolved
core/src/env/traits.rs Outdated Show resolved Hide resolved
core/src/env/api.rs Outdated Show resolved Hide resolved
Robbepop
Robbepop reviewed Apr 15, 2019
View reviewed changes
Copy link
Collaborator

@Robbepop Robbepop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks a lot better now.
Also will be easier to convert to node-runtime once that's done.

core/src/env/srml/types.rs Show resolved Hide resolved
core/src/env/srml/types.rs Outdated Show resolved Hide resolved
core/src/env/test_env.rs Outdated Show resolved Hide resolved
@shawntabrizi shawntabrizi mentioned this pull request Apr 15, 2019
Closed
Robbepop
Robbepop reviewed Apr 16, 2019
View reviewed changes
core/src/env/test_env.rs Outdated Show resolved Hide resolved
core/src/env/srml/types.rs Outdated Show resolved Hide resolved
Robbepop
Robbepop reviewed Apr 17, 2019
View reviewed changes
core/src/env/srml/types.rs Outdated
pub struct Address([u8; 32]);

impl Address {
pub fn new(address: [u8; 32]) -> Address {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For implementations like these it is recommended to just implement From<[u8;32]>.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for Hash

Robbepop
Robbepop approved these changes Apr 17, 2019
View reviewed changes
Copy link
Collaborator

@Robbepop Robbepop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Robbepop
Copy link
Collaborator

Robbepop commented Apr 17, 2019 •
edited
Loading

Sorry, didn't mean to close the PR ...
I merge as soon as CI is happy with it.

@Robbepop Robbepop closed this Apr 17, 2019
@Robbepop Robbepop reopened this Apr 17, 2019
@shawntabrizi shawntabrizi changed the title [core] Add ext_random_seed runtime call bindings [core,model] Add ext_random_seed runtime call bindings Apr 17, 2019
@Robbepop
Copy link
Collaborator

I only see formatting issues in the CI not related to this PR so I will merge this now.

@Robbepop Robbepop merged commit 5bf9b82 into use-ink:master Apr 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Robbepop Robbepop Robbepop approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement ext_random_seed
2 participants
@shawntabrizi @Robbepop