test staking #173

4meta5 · 2021-01-09T19:05:25Z

What does it do?

This is a follow up of #159 that builds on #162 . The primary goal of this PR is to automate calling set_author by every block producer when they produce a block so the author is necessarily set before every block is finalized.

add back requirement for a block author set every block
integration tests work with this requirement (and automatically set author every block)
integration test for author reporting => payment

What important points reviewers should know?

Is there something left for follow-up PRs?

#179
#180

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

What value does it bring to the blockchain users?

Checklist

Does it require a purge of the network? No
You bumped the runtime version if there are breaking changes in the runtime ?
Does it require changes in documentation/tutorials ? No

node/parachain/src/service.rs

…chain node

node/standalone/src/service.rs

pallets/author-inherent/src/lib.rs

…nbeam into amar-staking-tests

JoshOrndorff

I think this looks good and is ready. I left some comments below, but most of them were me learning out loud.

JoshOrndorff · 2021-01-14T13:30:02Z

node/parachain/src/chain_spec.rs

-		stake: Some(StakeConfig { stakers: vec![] }),
+		stake: Some(StakeConfig {
+			stakers: endowed_accounts
+				.iter()
+				.cloned()
+				.map(|k| (k, None, 100_000))
+				.collect(),
+		}),


So all of the endowed accounts are the initial stakers.

That means when we launch a parachain we have to give a particular CLI flag to include one of those accounts in the author inherent right? Can you give an example of what the collator command would look like?

So all of the endowed accounts are the initial stakers.

Yes, that is correct.

we have to give a particular CLI flag to include one of those accounts in the author inherent right?

I guess presumably just like the node CLI which now has the --account-id flag.

Can you give an example of what the collator command would look like?

Collator command to do what? To join the collator set or to author blocks?

Imagine a new user hears of moonbeam and wants to try it out. They decide to run a simple standalone node. So the run moonbase-standalone --dev. But now their chain doesn't work because no blocks meet the author inherent requirement. Doesn't it have to be moonbase-standalone --dev --author-id ????

I think this should go with the --dev/alice/bob work

node/parachain/src/cli.rs

node/parachain/src/command.rs

node/parachain/src/service.rs

node/standalone/src/chain_spec.rs

pallets/stake/src/lib.rs

runtime/src/lib.rs

tests/tests/test-balance.ts

JoshOrndorff · 2021-01-14T14:07:31Z

tests/tests/util/testWithMoonbeam.ts

@@ -46,6 +47,7 @@ export async function startMoonbeamNode(
    `--no-telemetry`,
    `--no-prometheus`,
    `--manual-seal`,
+    `--account-id=${GENESIS_ACCOUNT.substring(2)}`, // Required by author inherent


It would be nice to wire up the --dev --alice --bob etc flags to work with the author inherent. I'm okay if that happens later though.

Co-authored-by: Joshy Orndorff <JoshOrndorff@users.noreply.github.com>

crystalin

Awesome work !! 🔥
I left few small comments

pallets/stake/src/lib.rs

runtime/src/lib.rs

tests/tests/test-balance.ts

tests/tests/test-stake.ts

crystalin · 2021-01-14T17:51:01Z

It is also missing to include the --author in the scripts and docker images I believe, but I can take care of that in another PR

pallets/author-inherent/src/lib.rs

… None

JoshOrndorff

This is really good. I think we identified a few little things to clean up, but I think they should be in followups so we can start experimenting with this work more easily. I support merging.

4meta5 · 2021-01-15T13:50:44Z

Remaining TODOs:

fix --dev command
update script and docker images

tests/tests/test-balance.ts

tests/tests/test-stake.ts

JoshOrndorff and others added 23 commits January 6, 2021 12:53

WIP draft compiles

292cf70

Add authorship pallet.

4a0bd9e

bump spec version

960dbcf

cargo fmt

1341315

remove commented code

833fcea

add license

3f65512

wrap commetn

85d5b38

Add some notes

f8ae1e1

another idea

6ebf1a6

rename pallet

e32ad7b

remove pallet authorship. That was a false lead.

0676139

Make it compile

229a275

Event and event handler

07903dd

Ditch FindAuthor implementation.

6b2b1d8

Inherent is required

ca6dd11

Add the digest. Is it really that easy?

d13db17

Wire into standalone node. Lots of duplicate code.

047f1b7

Merge branch 'master' into joshy-authorship-inherent

ad6a5d3

cargo fmt

b01aa44

cleanup and fix

c4a7db5

Add to both runtimes

4eb7574

init

9b3468e

merge joshy-author-inherent into amar-staking-tests

2b3946a

4meta5 commented Jan 11, 2021

View reviewed changes

node/parachain/src/service.rs Outdated Show resolved Hide resolved

4meta5 added 3 commits January 10, 2021 20:06

add check-inherent impl and fix runtime config

fb38c04

add Call to stake config and fix standalone Cargo lock

0226a1e

add stakers in genesis and remove repeated timestamp inherent in para…

366291f

…chain node

4meta5 commented Jan 11, 2021

View reviewed changes

node/standalone/src/service.rs Outdated Show resolved Hide resolved

4meta5 commented Jan 11, 2021

View reviewed changes

pallets/author-inherent/src/lib.rs Outdated Show resolved Hide resolved

wired parachain cli with account id param

564d57e

4meta5 added 7 commits January 13, 2021 17:02

test rewards sent to sole validator for block authoring

400e0da

add code docs

c80f20a

Merge branch 'master' into amar-staking-tests

6fee604

green

139db40

Merge branch 'amar-staking-tests' of https://github.com/PureStake/moo…

e927a64

…nbeam into amar-staking-tests

fmt

ab6f7fe

Merge branch 'master' into amar-staking-tests

6b1a931

4meta5 added the A0-pleasereview Pull request needs code review. label Jan 14, 2021

4meta5 mentioned this pull request Jan 14, 2021

staking todos #179

Closed

JoshOrndorff approved these changes Jan 14, 2021

View reviewed changes

4meta5 and others added 6 commits January 14, 2021 12:33

Update tests/tests/test-balance.ts

c9f2920

Co-authored-by: Joshy Orndorff <JoshOrndorff@users.noreply.github.com>

Update pallets/author-inherent/src/lib.rs

15bc146

Co-authored-by: Joshy Orndorff <JoshOrndorff@users.noreply.github.com>

Update pallets/author-inherent/src/lib.rs

6039c3e

Co-authored-by: Joshy Orndorff <JoshOrndorff@users.noreply.github.com>

Update pallets/stake/src/lib.rs

26e18c6

Co-authored-by: Joshy Orndorff <JoshOrndorff@users.noreply.github.com>

Update pallets/stake/src/lib.rs

04eefe1

Co-authored-by: Joshy Orndorff <JoshOrndorff@users.noreply.github.com>

fmt and add back parachain import

ea3e1f6

crystalin approved these changes Jan 14, 2021

View reviewed changes

address some comments

30b0819

4meta5 commented Jan 14, 2021

View reviewed changes

pallets/author-inherent/src/lib.rs Outdated Show resolved Hide resolved

4meta5 added 3 commits January 14, 2021 15:21

comment use of account as H160 default in when not needed for call

f7abd5d

only register author inherent when running node and otherwise pass in…

0098715

… None

rm outdated comment

73b8f30

4meta5 requested a review from JoshOrndorff January 14, 2021 23:47

JoshOrndorff approved these changes Jan 15, 2021

View reviewed changes

4meta5 merged commit 22c4742 into master Jan 15, 2021

4meta5 deleted the amar-staking-tests branch January 15, 2021 13:47

JoshOrndorff mentioned this pull request Jan 18, 2021

Changes spec/wasm engine for run scripts #178

Merged

joelamouche reviewed Jan 19, 2021

View reviewed changes

tests/tests/test-balance.ts Show resolved Hide resolved

tests/tests/test-balance.ts Show resolved Hide resolved

tests/tests/test-stake.ts Show resolved Hide resolved

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test staking #173

test staking #173

4meta5 commented Jan 9, 2021 •

edited

JoshOrndorff left a comment

JoshOrndorff Jan 14, 2021

4meta5 Jan 14, 2021

JoshOrndorff Jan 15, 2021

crystalin Jan 15, 2021

JoshOrndorff Jan 14, 2021

crystalin left a comment

crystalin commented Jan 14, 2021

JoshOrndorff left a comment

4meta5 commented Jan 15, 2021 •

edited

test staking #173

test staking #173

Conversation

4meta5 commented Jan 9, 2021 • edited

What does it do?

What important points reviewers should know?

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

What value does it bring to the blockchain users?

Checklist

JoshOrndorff left a comment

Choose a reason for hiding this comment

JoshOrndorff Jan 14, 2021

Choose a reason for hiding this comment

4meta5 Jan 14, 2021

Choose a reason for hiding this comment

JoshOrndorff Jan 15, 2021

Choose a reason for hiding this comment

crystalin Jan 15, 2021

Choose a reason for hiding this comment

JoshOrndorff Jan 14, 2021

Choose a reason for hiding this comment

crystalin left a comment

Choose a reason for hiding this comment

crystalin commented Jan 14, 2021

JoshOrndorff left a comment

Choose a reason for hiding this comment

4meta5 commented Jan 15, 2021 • edited

4meta5 commented Jan 9, 2021 •

edited

4meta5 commented Jan 15, 2021 •

edited