Crowdloan rewards precompiles #607
Conversation
| // Solidity's bool type is 256 bits as shown by these examples | ||
| // https://docs.soliditylang.org/en/v0.8.0/abi-spec.html | ||
| // This utility function converts a Rust bool into the corresponding Solidity type | ||
| fn bool_to_solidity_bytes(b: bool) -> Vec<u8> { |
There was a problem hiding this comment.
We should really DRY this (I think it's copied here from parachain-staking)...
There was a problem hiding this comment.
I am just thinking of making a precompiles-common package that we can import here
This reverts commit 9a3ce46.
girazoki
added
the
B7-runtimenoteworthy
| return Err(ExitError::OutOfGas); | ||
| } | ||
| } | ||
| log::trace!(target: "crowdloan-rewards-precompile", "Made it past gas check"); |
There was a problem hiding this comment.
We could probably clean up some of these log statements before merging
| Ok(post_info) => { | ||
| let gas_used = Runtime::GasWeightMapping::weight_to_gas( | ||
| post_info.actual_weight.unwrap_or(info.weight), | ||
| ); |
There was a problem hiding this comment.
Is it possible that gas_used here could be > gas provided? Or is this guaranteed to be the same as required_gas above?
There was a problem hiding this comment.
I have been checking that substrate uses the get_dispatch_info to calculate weights in pallets like utility and others. So I assume they should coincide
| "In update_reward_address dispatchable wrapper" | ||
| ); | ||
| log::trace!(target: "crowdloan-rewards-precompile", "input is {:?}", input); | ||
| let new_address = parse_account(&input[..32])?; |
There was a problem hiding this comment.
Do you need a bounds check here?
There was a problem hiding this comment.
I can add them yes
| // Solidity's bool type is 256 bits as shown by these examples | ||
| // https://docs.soliditylang.org/en/v0.8.0/abi-spec.html | ||
| // This utility function converts a Rust bool into the corresponding Solidity type | ||
| pub fn bool_to_solidity_bytes(b: bool) -> Vec<u8> { |
There was a problem hiding this comment.
Perhaps this could be a const fn and could be something like:
match b {
true => 1usize.into(),
false => 0usize.into(),
}Assuming it could return a U256...
There was a problem hiding this comment.
And then convert the U256 to vec?
There was a problem hiding this comment.
Maybe. Isn't that the purpose of the following fn?
There was a problem hiding this comment.
Yeah perhaps is more clear that way
There was a problem hiding this comment.
having constant functions does not allow to do function calls or conversions, so I guess I will leave it as it is for now
This reverts commit b886737.
What does it do?
It adds crowdloan precompiles for:
This PR follows closely the test structure in #518. Ideally once any of these gets merged, we should move some test utils to precompiles-utils
What important points reviewers should know?
Is there something left for follow-up PRs?
What alternative implementations were considered?
Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?
What value does it bring to the blockchain users?
Checklist