Add Musicoin and MCIP-3 UBI hardfork. #6621

5chdn · 2017-10-02T14:46:26Z

What:

--chain musicoin logic for core and wallet
ethash logic for MCIP-3 transition scheduled for block 1_200_001

Rationale:

eliminates the need for musicoin to distribute their own fork of parity

5chdn · 2017-10-02T14:47:55Z

Tested MCIP-3 transition against GMC 2.0.0

tomusdrw · 2017-10-02T14:57:18Z

ethcore/src/ethereum/ethash.rs

 		let result_block_reward = reward + reward.shr(5) * U256::from(n_uncles);
 		let mut uncle_rewards = Vec::with_capacity(n_uncles);

-		self.machine.add_balance(block, &author, &result_block_reward)?;
+		if number >= self.ethash_params.mcip3_transition {


Is it possible to write tests for that?

Yes, would be lucky to do that. Do I just add tests at the bottom of ethash.rs? Can I reuse the test_spec() or should I create a new test chain that includes the transition? Edit: Created tests for the block rewards.

rphmeier · 2017-10-02T15:06:13Z

ethcore/src/ethereum/ethash.rs

+			let ubi_contract = self.ethash_params.mcip3_ubi_contract;
+			let ubi_reward = self.ethash_params.mcip3_ubi_reward.unwrap();
+			let dev_contract = self.ethash_params.mcip3_dev_contract;
+			let dev_reward = self.ethash_params.mcip3_dev_reward.unwrap();


we don't use unwrap(). favor expect() with a proof that it will never panic. That these fields are either all set or all not should be checked at spec parsing time. Grouping them into a struct mcip3 parsed optionally would make that logic a lot easier.

This makes sense. I have rewritten the logic, now a malformed mcip3 spec will just behave like a default block reward scheme.

rphmeier · 2017-10-05T16:04:51Z

ethcore/src/ethereum/ethash.rs

@@ -219,7 +252,7 @@ impl Engine<EthereumMachine> for Arc<Ethash> {
 			self.machine.add_balance(block, a, reward)?;
 		}

-		// note and trace.
+		// Note and trace.
 		self.machine.note_rewards(block, &[(author, result_block_reward)], &uncle_rewards)


without altering this line, block rewards from this change won't be traced. I would recommend using a
Cow<&'a [(Address, U256)]> to store the main block reward initially to avoid an allocation on every block unless it's a musicoin chain.

or a separate call to machine.note_rewards within the musicoin conditional would serve the same purpose.

Should I call machine.note_rewards for each block reward or only for the miner reward? Also, is the order of machine.add_balance important?

note_rewards takes two slices of &[(Address, U256)]. The first one is for block rewards and the second for uncle rewards. I don't think it matters if you make multiple calls to the function.

@grbIzl tracing does handle multiple block rewards correctly, right?

@rphmeier it should. It's pretty straightforward.

good. @5chdn then you would say the correct implementation traces only the miner reward and not any of the others? LGTM if so.

rphmeier · 2017-10-05T16:05:54Z

LGTM except for tracing nit

5chdn · 2017-10-06T09:43:26Z

Reused result_block_reward variable to make sure the correct reward is traced. Should be all now.

5chdn added 23 commits September 20, 2017 13:03

Add musicoin chain spec.

ec65195

Add musicoin to parity node

71b5108

Add musicoin to the wallet

e8d519d

Add i18n for musicoin

ce70df4

Align musicoin chain spec with 1.8, ref #6134

fef6698

Update musicoin bootnodes

5dc2a33

Merge branch 'master' into a5-chains-musicoin

3442144

Prepare MCIP-3 in musicoin chain spec.

41dfbfc

Update musicoin chain spec with contract addresses for MCIP-3

2c2ed16

Extend ethash params by MCIP-3

d3831af

Fix musicoin chain spec json

07dbdb8

Use U256 for block rewards.

7940bf6

Update musicoin registrar

7ca83d9

Merge master

fa65407

Fix merge leftovers

1cfbea4

Update musicoin chain spec for latest master

77a3ba8

Bestow MCIP-3 block reward(s).

f292915

Update musicoin registry once and for all

44ca37f

Merge upstream

d2ecacf

Align MCIP-3 block reward with go implementation

939354c

Remove mcip3 test chain spec from repository

36a852a

Update MCIP-3 block rewards

1b9cdfb

Merge branch 'master' into a5-chains-musicoin

6c087c2

5chdn added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. M7-ui labels Oct 2, 2017

tomusdrw reviewed Oct 2, 2017

View reviewed changes

rphmeier reviewed Oct 2, 2017

View reviewed changes

5chdn added the A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. label Oct 2, 2017

5chdn added 8 commits October 3, 2017 12:14

Giving mcip3 rewards a useful default value.

8319991

Fix ethjson tests.

d7db848

Update musicoin chain spec

b43006e

Merge branch 'master' into a5-chains-musicoin

d747307

Fix tests 0:)

12cd04c

Add musicoin mcip3 era test spec.

d0d8f1c

Update musicoin chain spec(s)

babb3a3

Add tests for mcip3 era block rewards

f49aa0b

5chdn added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Oct 4, 2017

5chdn added 2 commits October 4, 2017 18:45

Merge branch 'master' into a5-chains-musicoin

559d9a9

Fix tests

4e9e8c9

5chdn added this to the 1.8 milestone Oct 5, 2017

arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Oct 5, 2017

rphmeier reviewed Oct 5, 2017

View reviewed changes

rphmeier added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Oct 5, 2017

5chdn added 3 commits October 6, 2017 09:13

Merge branch 'master' into a5-chains-musicoin

21f27b5

Disable byzantium for musicoin

5a21744

Pass miner reward to the tracer.

0cb6303

Allow modifying blockreward in MCIP-3 transition.

dd36b4c

rphmeier added A8-looksgood 🦄 Pull request is reviewed well. and removed A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Oct 6, 2017

gavofyork merged commit 360ecd3 into master Oct 8, 2017

gavofyork deleted the a5-chains-musicoin branch October 8, 2017 16:18

5chdn added the B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. label Oct 8, 2017

5chdn mentioned this pull request Nov 27, 2017

Byzantium Update for Expanse #7146

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Musicoin and MCIP-3 UBI hardfork. #6621

Add Musicoin and MCIP-3 UBI hardfork. #6621

5chdn commented Oct 2, 2017

5chdn commented Oct 2, 2017

tomusdrw Oct 2, 2017

5chdn Oct 4, 2017 •

edited

Loading

rphmeier Oct 2, 2017 •

edited

Loading

5chdn Oct 4, 2017

rphmeier Oct 5, 2017

rphmeier Oct 5, 2017 •

edited

Loading

5chdn Oct 5, 2017 •

edited

Loading

rphmeier Oct 6, 2017

grbIzl Oct 6, 2017

rphmeier Oct 6, 2017

rphmeier commented Oct 5, 2017

5chdn commented Oct 6, 2017

Add Musicoin and MCIP-3 UBI hardfork. #6621

Add Musicoin and MCIP-3 UBI hardfork. #6621

Conversation

5chdn commented Oct 2, 2017

5chdn commented Oct 2, 2017

tomusdrw Oct 2, 2017

Choose a reason for hiding this comment

5chdn Oct 4, 2017 • edited Loading

Choose a reason for hiding this comment

rphmeier Oct 2, 2017 • edited Loading

Choose a reason for hiding this comment

5chdn Oct 4, 2017

Choose a reason for hiding this comment

rphmeier Oct 5, 2017

Choose a reason for hiding this comment

rphmeier Oct 5, 2017 • edited Loading

Choose a reason for hiding this comment

5chdn Oct 5, 2017 • edited Loading

Choose a reason for hiding this comment

rphmeier Oct 6, 2017

Choose a reason for hiding this comment

grbIzl Oct 6, 2017

Choose a reason for hiding this comment

rphmeier Oct 6, 2017

Choose a reason for hiding this comment

rphmeier commented Oct 5, 2017

5chdn commented Oct 6, 2017

5chdn Oct 4, 2017 •

edited

Loading

rphmeier Oct 2, 2017 •

edited

Loading

rphmeier Oct 5, 2017 •

edited

Loading

5chdn Oct 5, 2017 •

edited

Loading