Skip to content

Commit

Permalink
Introduce inherent digests (#2466)
Browse files Browse the repository at this point in the history
* Introduce inherent digests

* Implement inherent digests

* fix silly error

* Implementation of inherent digests in BABE

All tests pass. There are still limitations:

1. The runtime strips out inherent digests, so BABE must re-add them.
2. The test runtime checks that it can re-compute all digests.  It
   can’t, so I had to comment out that test.

* Fix compilation and seal import

Seals were not imported correctly: the pre-digest was imported twice,
instead of both it and the seal being imported.  Also, other parts of
the code did not compile due to incomplete refactoring.

* Remove bogus assertion

* Fix testsuite compilation

* Remove unused import

* Fix compiler diagnostics

* Add inherent digest parameters to block constructors

This enforces that inherent digests are added first.

* Fixup Cargo.lock

* Fix build errors

* Re-add an incorrectly removed import

* Bump primitive-types version

* Update Cargo.lock

* Refactoring

* Use inherent digests for AuRa

They do reach the runtime, but get stripped.  I have not figured out
where.

* Fix compilation errors

* Fix compilation errors due to incorrect types

* Fix whitespace

Suggested-by: Tomasz Drwiega <tomasz@parity.io>

* Add preamble

Suggested-by: Tomasz Drwiega <tomasz@parity.io>

* Fix silly compile error

* Refactor pre-digest finding code into a separate function

* Remove unwanted assertion

It is too likely to bring down the entire blockchain.

Suggested-by: Tomasz Drwiega <tomasz@parity.io>

* Use `find_pre_digest` after runtime, too

Also, use `Member` trait rather than rolling our own requirements.

Suggested-by: Tomasz Drwiega <tomasz@parity.io>

* Fix various warnings

mostly due to upgrading the dependency on `error_chain`.

* Pre-digests nearly complete

This nearly completes the implementation of pre-runtime digests.

* `Seal2` → `Seal` and fix test suite

* Try to fix the storage error

* Try to fix storage (again)

* Fix tests

* Hopefully finish pre-runtime digests

The key is to pass *only* the pre-runtime digests to the runtime.  The
others must be stripped out by `initialize_block`.

* Fix silly typo

* Fix another silly mistake

* Remove unnecessary filtering of BABE pre-digests

We no longer get duplicate BABE pre-digests, so if they appear, the
header should be rejected outright.

* Update Cargo.lock files

* Reformatting

* Fix silly typo in inherent digest code

Also, revert `error.rs` files that contained calls to the `error_chain!`
macro.

* Try to keep the runtime from stripping pre-digests

Currently runs into the “Storage root must match that calculated”
assertion.

* Don’t compute storage root until storage changes are done.

Also, fix a compilation error.

* Fix compile-time error

* Fix compilation errors

* Fix more compile errors

* Hopefully it compiles this time…

* Fix compilation and add docs

* Prevent BABE from adding duplicate pre-runtime digests

Found by comparing with the AuRa code.  I also did some refactoring.

* Respond to review and fix some warnings

* Delete some dead code introduced earlier

* More dead code goes away

* `ref mut` → `&mut`

* Respond to review and fix some warnings

* Fix compilation error

* Remove unneeded `HashT` type parameter

Suggested-by: Robert Habermeier <robert@parity.io>

* Remove spurious #[allow(deprecated)]

* Document inherent digest parameter to `build_block`

* Delete `Simple` trait

It wasn’t needed

* delete wrongly added files

* Fix trait bounds

* Digest serialization tests

I also did some reformatting and cleanup.

* Apply suggestions from code review

Reformatting

Co-Authored-By: André Silva <andre.beat@gmail.com>

* Swap two arguments to `propose` and `propose_with`

Also, remove some needless unsafe code.

* Remove bogus `#![allow(deprecated)]` annotations

With the removal of the deprecated `Seal` variant, these are not needed.

* Add a missing `#[allow(deprecated)]` in the AuRa tests

* Fix silly compile error

* Fix silly compiler error

RLS did not tell me that I hadn’t fixed `babe/lib.rs`, so I missed it.

* Fixes made automatically by Cargo
  • Loading branch information
Demi-Marie authored and gavofyork committed May 29, 2019
1 parent e9a4c80 commit c7d1204
Show file tree
Hide file tree
Showing 55 changed files with 1,033 additions and 873 deletions.
4 changes: 2 additions & 2 deletions substrate/.gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ publish-docker-release:
- docker build
--build-arg VCS_REF="${CI_COMMIT_SHA}"
--build-arg BUILD_DATE="$(date -u '+%Y-%m-%dT%H:%M:%SZ')"
--tag $CONTAINER_IMAGE:$VERSION
--tag $CONTAINER_IMAGE:$VERSION
--tag $CONTAINER_IMAGE:latest .
- docker push $CONTAINER_IMAGE:$VERSION
- docker push $CONTAINER_IMAGE:latest
Expand Down Expand Up @@ -272,7 +272,7 @@ publish-s3-doc:
--human-readable --summarize

.deploy-template: &deploy
stage: kubernetes
stage: kubernetes
when: manual
retry: 1
image: parity/kubetools:latest
Expand Down
25 changes: 17 additions & 8 deletions substrate/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions substrate/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ RUN curl https://sh.rustup.rs -sSf | sh -s -- -y && \
rustup target add wasm32-unknown-unknown --toolchain nightly && \
cargo install --git https://github.com/alexcrichton/wasm-gc && \
rustup default nightly && \
./scripts/build.sh && \
rustup default stable && \
./scripts/build.sh && \
rustup default stable && \
cargo build --$PROFILE

# ===== SECOND STAGE ======
Expand Down
38 changes: 25 additions & 13 deletions substrate/core/basic-authorship/src/basic_authorship.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ use codec::Decode;
use consensus_common::{self, evaluation};
use primitives::{H256, Blake2Hasher, ExecutionContext};
use runtime_primitives::traits::{
Block as BlockT, Hash as HashT, Header as HeaderT, ProvideRuntimeApi, AuthorityIdFor
Block as BlockT, Hash as HashT, Header as HeaderT, ProvideRuntimeApi,
AuthorityIdFor, DigestFor,
};
use runtime_primitives::generic::BlockId;
use runtime_primitives::ApplyError;
Expand All @@ -53,11 +54,13 @@ pub trait AuthoringApi: Send + Sync + ProvideRuntimeApi where
/// The error used by this API type.
type Error: std::error::Error;

/// Build a block on top of the given, with inherent extrinsics pre-pushed.
/// Build a block on top of the given block, with inherent extrinsics and
/// inherent digests pre-pushed.
fn build_block<F: FnMut(&mut BlockBuilder<Self::Block>) -> ()>(
&self,
at: &BlockId<Self::Block>,
inherent_data: InherentData,
inherent_digests: DigestFor<Self::Block>,
build_ctx: F,
) -> Result<Self::Block, error::Error>;
}
Expand Down Expand Up @@ -92,9 +95,11 @@ impl<B, E, Block, RA> AuthoringApi for SubstrateClient<B, E, Block, RA> where
&self,
at: &BlockId<Self::Block>,
inherent_data: InherentData,
inherent_digests: DigestFor<Self::Block>,
mut build_ctx: F,
) -> Result<Self::Block, error::Error> {
let mut block_builder = self.new_block_at(at)?;

let mut block_builder = self.new_block_at(at, inherent_digests)?;

let runtime_api = self.runtime_api();
// We don't check the API versions any further here since the dispatch compatibility
Expand Down Expand Up @@ -174,12 +179,16 @@ impl<Block, C, A> consensus_common::Proposer<<C as AuthoringApi>::Block> for Pro
type Create = Result<<C as AuthoringApi>::Block, error::Error>;
type Error = error::Error;

fn propose(&self, inherent_data: InherentData, max_duration: time::Duration)
-> Result<<C as AuthoringApi>::Block, error::Error>
fn propose(
&self,
inherent_data: InherentData,
inherent_digests: DigestFor<Block>,
max_duration: time::Duration,
) -> Result<<C as AuthoringApi>::Block, error::Error>
{
// leave some time for evaluation and block finalization (33%)
let deadline = (self.now)() + max_duration - max_duration / 3;
self.propose_with(inherent_data, deadline)
self.propose_with(inherent_data, inherent_digests, deadline)
}
}

Expand All @@ -190,8 +199,12 @@ impl<Block, C, A> Proposer<Block, C, A> where
A: txpool::ChainApi<Block=Block>,
client::error::Error: From<<C as AuthoringApi>::Error>,
{
fn propose_with(&self, inherent_data: InherentData, deadline: time::Instant)
-> Result<<C as AuthoringApi>::Block, error::Error>
fn propose_with(
&self,
inherent_data: InherentData,
inherent_digests: DigestFor<Block>,
deadline: time::Instant,
) -> Result<<C as AuthoringApi>::Block, error::Error>
{
use runtime_primitives::traits::BlakeTwo256;

Expand All @@ -203,17 +216,16 @@ impl<Block, C, A> Proposer<Block, C, A> where
let block = self.client.build_block(
&self.parent_id,
inherent_data,
inherent_digests.clone(),
|block_builder| {
// Add inherents from the internal pool

let inherents = self.inherents_pool.drain();
debug!("Pushing {} queued inherents.", inherents.len());
debug!("Pushing {} queued inherent extrinsics.", inherents.len());
for i in inherents {
if let Err(e) = block_builder.push_extrinsic(i) {
warn!("Error while pushing inherent extrinsic from the pool: {:?}", e);
}
}

// proceed with transactions
let mut is_first = true;
let mut skipped = 0;
Expand Down Expand Up @@ -331,7 +343,7 @@ mod tests {
cell.replace(new)
});
let deadline = time::Duration::from_secs(3);
let block = proposer.propose(Default::default(), deadline).unwrap();
let block = proposer.propose(Default::default(), Default::default(), deadline).unwrap();

// then
// block should have some extrinsics although we have some more in the pool.
Expand Down Expand Up @@ -362,7 +374,7 @@ mod tests {

// when
let deadline = time::Duration::from_secs(3);
let block = proposer.propose(Default::default(), deadline).unwrap();
let block = proposer.propose(Default::default(), Default::default(), deadline).unwrap();

// then
assert_eq!(block.extrinsics().len(), 1);
Expand Down
2 changes: 1 addition & 1 deletion substrate/core/client/db/src/light.rs
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,7 @@ pub(crate) mod tests {
let mut prev_hash = insert_final_block(&db, HashMap::new(), || header_producer(&Default::default(), 0));

// insert SIZE blocks && ensure that nothing is pruned

for number in 0..cht::size() {
prev_hash = insert_block(&db, HashMap::new(), || header_producer(&prev_hash, 1 + number));
}
Expand Down
16 changes: 9 additions & 7 deletions substrate/core/client/src/block_builder/block_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use parity_codec::Encode;
use runtime_primitives::ApplyOutcome;
use runtime_primitives::generic::BlockId;
use runtime_primitives::traits::{
Header as HeaderT, Hash, Block as BlockT, One, HashFor, ProvideRuntimeApi, ApiRef
Header as HeaderT, Hash, Block as BlockT, One, HashFor, ProvideRuntimeApi, ApiRef, DigestFor,
};
use primitives::{H256, ExecutionContext};
use crate::blockchain::HeaderBackend;
Expand All @@ -41,10 +41,11 @@ where
A: ProvideRuntimeApi + HeaderBackend<Block> + 'a,
A::Api: BlockBuilderApi<Block>,
{
/// Create a new instance of builder from the given client, building on the latest block.
pub fn new(api: &'a A) -> error::Result<Self> {
/// Create a new instance of builder from the given client, building on the
/// latest block.
pub fn new(api: &'a A, inherent_digests: DigestFor<Block>) -> error::Result<Self> {
api.info().and_then(|i|
Self::at_block(&BlockId::Hash(i.best_hash), api, false)
Self::at_block(&BlockId::Hash(i.best_hash), api, false, inherent_digests)
)
}

Expand All @@ -57,7 +58,8 @@ where
pub fn at_block(
block_id: &BlockId<Block>,
api: &'a A,
proof_recording: bool
proof_recording: bool,
inherent_digests: DigestFor<Block>,
) -> error::Result<Self> {
let number = api.block_number_from_id(block_id)?
.ok_or_else(|| error::Error::UnknownBlock(format!("{}", block_id)))?
Expand All @@ -70,7 +72,7 @@ where
Default::default(),
Default::default(),
parent_hash,
Default::default()
inherent_digests,
);

let mut api = api.runtime_api();
Expand All @@ -80,7 +82,7 @@ where
}

api.initialize_block_with_context(
block_id, ExecutionContext::BlockConstruction, &header
block_id, ExecutionContext::BlockConstruction, &header,
)?;

Ok(BlockBuilder {
Expand Down
Loading

0 comments on commit c7d1204

Please sign in to comment.