Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Prevent building invalid blocks #430

Merged
merged 8 commits into from
Jul 27, 2018
Merged

Prevent building invalid blocks #430

merged 8 commits into from
Jul 27, 2018

Conversation

arkpar
Copy link
Member

@arkpar arkpar commented Jul 26, 2018

  • apply_extrinsic return value is now considered by the block builder.
  • Removed panic=abort since native runtime requires unwinding.

@arkpar arkpar added A3-in_progress Pull request is in progress. No review needed at this stage. A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Jul 26, 2018
Cargo.toml Outdated
@@ -89,6 +89,3 @@ travis-ci = { repository = "paritytech/polkadot", branch = "master" }
maintenance = { status = "actively-developed" }
is-it-maintained-issue-resolution = { repository = "paritytech/polkadot" }
is-it-maintained-open-issues = { repository = "paritytech/polkadot" }

[profile.release]
panic = "abort"
Copy link
Contributor

Choose a reason for hiding this comment

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

Even though panic=unwind is the default, I'd rather specify it explicitly with a comment for documentation purposes.

@@ -111,6 +118,12 @@ error_chain! {
description("remote fetch failed"),
display("Remote data fetch has been failed"),
}

/// Error decoding extrinsic outcome.
ApplyExtinsicFailed {
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. I'm not sure why this is called ApplyExtinsicFailed? I think it is more like ApplyResultDecodeFailed
  2. Why the comment, description and display messages refers to an outcome and not to a result?

Runtime(e: ApplyError) {
description("Extrinsic error"),
display("Extrinsic error: {:?}", e),
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we rename this to something more extrinsic-related. We could use ApplyExtinsicFailed for this IMO.

@gavofyork
Copy link
Member

Would be nice to know why the transaction queue/block builder was letting blatantly invalid transactions through at all...

@gavofyork gavofyork added A8-looksgood and removed A0-please_review Pull request needs code review. labels Jul 27, 2018
@arkpar
Copy link
Member Author

arkpar commented Jul 27, 2018

Transaction pool does not check for insufficient balance currently. And for signature correctness as far as I can see.
@tomusdrw Is this right?

@arkpar arkpar merged commit 9feedaa into master Jul 27, 2018
@arkpar arkpar deleted the ark-block-builder-fix branch July 27, 2018 09:11
dvdplm added a commit that referenced this pull request Jul 30, 2018
* master: (86 commits)
  Make contract a separate runtime module (#345)
  Version bump (#450)
  DB-based blockchain data cache for light nodes (#251)
  Update libp2p again (#445)
  Update version on git head change (#444)
  Fix the public key of bootnode 3 (#441)
  Update libp2p (#442)
  Switch to the master branch of libp2p (#427)
  Export ws port 9944 and add doc (#440)
  Iterate over overlay to decide which keys to purge (#436)
  Exit signal gets its own trait (#433)
  Add docker image (#375)
  Reset peers.json if the content is not loadable (#405)
  Limit number of incoming connections (#391)
  Fix memory leaks in libp2p (#432)
  Do not queue empty blocks set for import (#431)
  5 random fixes (#1) (#435)
  Chore: fix typo (#434)
  Prevent building invalid blocks (#430)
  Better logging for public key mismatch (#429)
  ...
lamafab pushed a commit to lamafab/substrate that referenced this pull request Jun 16, 2020
* trigger pipeline in github-api for polkasync

* update polkadot branch name for replacement

* deployment: update project
liuchengxu added a commit to chainx-org/substrate that referenced this pull request Aug 23, 2021
liuchengxu pushed a commit to autonomys/substrate that referenced this pull request Jun 3, 2022
…tor-part-2

Refactor cirrus executor (part 2)
helin6 pushed a commit to boolnetwork/substrate that referenced this pull request Jul 25, 2023
* test-runtime: Fix README typo

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* test-runtime: Explicit error handling for missing substrate binary

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* test-runtime: Fix documentation typo

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* events: Test primitive decode_and_consume

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* events: Test tuple decode_and_consume

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* events: Test array decode_and_consume

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* events: Extend array with sequences

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* events: Test variant decode_and_consume

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* events: Test composite decode_and_consume

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* events: Test compact decode_and_consume

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants