-
Notifications
You must be signed in to change notification settings - Fork 431
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
Temporarily add Determinism
enum from pallet-contracts
#1547
Conversation
Determinism
enum from pallet-contracts
Determinism
enum from pallet-contracts
Codecov Report
@@ Coverage Diff @@
## master #1547 +/- ##
==========================================
- Coverage 71.57% 71.55% -0.03%
==========================================
Files 205 205
Lines 6294 6296 +2
==========================================
Hits 4505 4505
- Misses 1789 1791 +2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@@ -16,7 +16,7 @@ pub mod flipper { | |||
|
|||
/// Creates a new flipper smart contract initialized to `false`. | |||
#[ink(constructor)] | |||
pub fn default() -> Self { | |||
pub fn new_default() -> Self { |
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.
Why are the new_default
changes necessary?
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.
New clippy lints do not allow methods with default()
name as they can be confused with the Default::default()
implementation. See the the previous clippy output: https://gitlab.parity.io/parity/mirrors/ink/-/jobs/2171790
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.
It looks like this lint's been around a very long time, why is it only being triggered now?
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 not a fan of this new_default
method, it made sense to have a default
method for the contract. Maybe we can work around it using a the Default
trait impl instead?
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 not a fan of this
new_default
method, it made sense to have adefault
method for the contract. Maybe we can work around it using a theDefault
trait impl instead?
Can you open a follow up issue for this please?
@@ -28,7 +28,7 @@ variables: | |||
CARGO_TARGET_DIR: "/ci-cache/${CI_PROJECT_NAME}/targets/${CI_COMMIT_REF_NAME}/${CI_JOB_NAME}" | |||
# CI_IMAGE is changed to "-:staging" when the CI image gets rebuilt | |||
# read more https://github.com/paritytech/scripts/pull/244 | |||
CI_IMAGE: "paritytech/ink-ci-linux:production" | |||
CI_IMAGE: "paritytech/ink-ci-linux:staging" |
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 should've been rolled back before merging
#[derive( | ||
Debug, Clone, Copy, scale::Encode, scale::Decode, PartialEq, Eq, serde::Serialize, | ||
)] | ||
pub enum Determinism { |
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 is missing docs
* copy Determinism enum from pallet and add it tx calls * Use `staging` image * apply clippy suggestions * rename default constructors to new_default * fix UI test error message Co-authored-by: Hernando Castano <hernando@hcastano.com>
Recent release of substrate-contracts-node introduce new parameter
determinism
inupload_code()
which needs to be specified on the ink! and cargo-contract sides, otherwise no E2E tests can be run with the latest substrate-contracts-node version.pallet-contracts
currently do not exposepallet_contracts::Determinism
publicly.Therefore, the temporary solution is to copy-paste the enum into the ink! codebase until the next release of
polkadot
branch in substrate and synchronisation of substrate-contracts-node