-
Notifications
You must be signed in to change notification settings - Fork 516
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.
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
Moves precompile-utils from moonbeam into frontier. #1150
Moves precompile-utils from moonbeam into frontier. #1150
Conversation
Cargo.toml
Outdated
"utils/precompiles", | ||
"utils/precompiles/macro", | ||
"utils/precompiles/tests-external", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"utils/precompiles", | |
"utils/precompiles/macro", | |
"utils/precompiles/tests-external", | |
"precompiles", | |
"precompiles/macro", | |
"precompiles/tests-external", |
This utils
path seems unnecessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Cargo.toml
Outdated
rlp = { version = "0.5.2", default-features = false } | ||
scale-codec = { package = "parity-scale-codec", version = "3.6.4", default-features = false, features = ["derive"] } | ||
scale-info = { version = "2.9.0", default-features = false, features = ["derive"] } | ||
serde = { version = "1.0", default-features = false, features = ["derive", "alloc"] } | ||
serde_json = "1.0" | ||
sha3 = { version = "0.10", default-features = false } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sha3 = { version = "0.10", default-features = false } |
we could use sp_core::hashing
directly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Co-authored-by: Qinxuan Chen <[email protected]>
Co-authored-by: Qinxuan Chen <[email protected]>
// GNU General Public License for more details. | ||
|
||
// You should have received a copy of the GNU General Public License | ||
// along with Moonbeam. If not, see <http://www.gnu.org/licenses/>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to update these licence headers? Keep it the same as other files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks! Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unlike individual contributors, I don't think "PureStake Inc" signed any CLA with Parity, so we can't just remove the previous license headers. The previous attribution should be kept, so the end result looks something like this:
// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0
// This file is part of Frontier.
//
// Copyright (c) 2019-2022 PureStake Inc.
// Copyright (c) 2023 Parity Technologies (UK) Ltd.
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with Wei's result but I'll get confirmation from the Moonbeam Foundation (PureStake Inc. will cease to exist at some point)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confirm it is fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but the code probably better belongs in frame/evm/precompile
.
If we move the code to And the introduced precompile macro in this PR is more used for customization, while the precompiles in |
.pre-commit-config.yaml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file seems to be your local development configuration? @rimbi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, thanks! Removed.
It's not a big deal anyway. We can also move |
What does it do?
Moves precompile-utils from moonbeam into frontier.
precompile-utils
precompile-utils-macro
precompile-utils-tests-external