Skip to content
This repository has been archived by the owner on Feb 21, 2022. It is now read-only.

Synchronize with Substrate #71

Merged
merged 5 commits into from
Aug 25, 2021
Merged

Conversation

cmichi
Copy link
Contributor

@cmichi cmichi commented Aug 24, 2021

I've also reduced the diff to the Substrate node-template, hence why there are some non-functional changes. This makes it easier to keep our canvas-node in sync.

@cmichi cmichi requested a review from ascjones August 24, 2021 09:43
@athei
Copy link
Member

athei commented Aug 24, 2021

Should be resynced once paritytech/substrate#9592 is merged to be able to run on rococo.

pub BlockWeights: frame_system::limits::BlockWeights = frame_system::limits::BlockWeights
::with_sensible_defaults(2 * WEIGHT_PER_SECOND, NORMAL_DISPATCH_RATIO);
pub BlockLength: frame_system::limits::BlockLength = frame_system::limits::BlockLength
::max_with_normal_ratio(5 * 1024 * 1024, NORMAL_DISPATCH_RATIO);
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't remember the exact reasoning (maybe @athei remembers), but BlockWeights and BlockLength definitions were copied from the default node-runtime.

Copy link
Member

@athei athei Aug 24, 2021

Choose a reason for hiding this comment

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

Cause the DeletionWeightLimit and DeletionQueueDepth depend on that to parameterize the lazy contract deletion. We should really push for solving this issue at the database level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, I've added it back with a comment pointing this out.

Copy link
Contributor

@HCastano HCastano left a comment

Choose a reason for hiding this comment

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

I'm okay getting this in now and doing another smaller update once paritytech/substrate#9592 is merged

#!/bin/bash

# Helper script which outputs the diff to Substrate's `node-template`.
SUBSTRATE_DIR=${SUBSTRATE_DIR:-~/projects/substrate}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd change this to be user provided since not everyone has their Substrate build in ~/projects/substrate 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It already does this, this is just the default. The bash syntax is ${VAR:-default}.

So you can invoke the script with SUBSTRATE_DIR=/tmp/substrate substrate-diff.sh.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, but imo it's not a great default. Having no default and enforcing that users point to their Substrate folder would be better

@cmichi cmichi merged commit d44ee14 into master Aug 25, 2021
@cmichi cmichi deleted the cmichi-sync-with-substrate-20210824 branch August 25, 2021 07:32
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.

4 participants