Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Miner property to IActionContext #174

Merged
merged 2 commits into from Apr 4, 2019

Conversation

Projects
None yet
4 participants
@earlbread
Copy link
Member

commented Apr 4, 2019

  • Added Miner property to IActionContext.
  • Renamed Block<T>.RewardBeneficiary to Block<T>.Miner
    Resolves #173

@earlbread earlbread self-assigned this Apr 4, 2019

@earlbread earlbread requested review from dahlia and longfin Apr 4, 2019

@earlbread earlbread force-pushed the earlbread:add-miner-to-action-context branch from 6c0a1c0 to ff3e7ee Apr 4, 2019

@kijun

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

Perhaps we should call miner block producer instead? I don't have a strong opinion yet, but thinking we might rely on PoA or PoS to produce blocks in the future.

@dahlia

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

Perhaps we should call miner block producer instead? I don't have a strong opinion yet, but thinking we might rely on PoA or PoS to produce blocks in the future.

This term (“mine” or “miner”) is currently used in everywhere, e.g., BlockChain<T>.MineBlock(), Block<T>.Mine(). Even if we decide to reword them we'd better do that in a separated patch.

@kijun

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

Makes sense!

@@ -173,7 +173,8 @@ public byte[] ToBencodex(bool hash, bool transactionData)
);
foreach (Transaction<T> tx in Transactions)
{
delta = tx.EvaluateActions(Hash, Index, delta);
delta = tx.EvaluateActions(
Hash, Index, delta, RewardBeneficiary.Value);

This comment has been minimized.

Copy link
@longfin

longfin Apr 4, 2019

Member

It seems that rewardBeneficiary is no longer a proper name anymore, how about fixing it this time?

This comment has been minimized.

Copy link
@dahlia

dahlia Apr 4, 2019

Member

I'm for this proposal. How about Block<T>.Miner?

This comment has been minimized.

Copy link
@earlbread

earlbread Apr 4, 2019

Author Member

I agree. Block<T>.Miner seems good.

@dahlia

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

FYI:

$ rg -i '\bminer?\b|\bminer?[A-Z]'
Libplanet.Tests/TestUtils.cs
78:        internal static Block<T> MineGenesis<T>()
96:        internal static Block<T> MineNext<T>(
116:                return Block<T>.Mine(

Libplanet/Action/IActionContext.cs
45:        /// then reproduce the same result, while neither a single block miner

Libplanet.Tests/Blocks/BlockFixture.cs
16:            Genesis = TestUtils.MineGenesis<PolymorphicAction<BaseAction>>();
17:            Next = TestUtils.MineNext(
25:            HasTx = TestUtils.MineNext(

Libplanet/Blockchain/BlockChain.cs
232:        public Block<T> MineBlock(
254:                Block<T> block = Block<T>.Mine(
272:        public Block<T> MineBlock(Address rewardBeneficiary) =>
273:            MineBlock(rewardBeneficiary, DateTimeOffset.UtcNow);
406:        // ALL blocks (referenced by MineBlock(), etc.) will be changed.

Libplanet/Blocks/Block.cs
104:        public static Block<T> Mine(

Libplanet/Tx/Transaction.cs
274:        /// a <see cref="Libplanet.Blocks.Block{T}"/> is mined.

Libplanet.Tests/Store/FileStoreFixture.cs
77:            Block1 = TestUtils.MineGenesis<DumbAction>();
78:            Block2 = TestUtils.MineNext(Block1);
79:            Block3 = TestUtils.MineNext(Block2);

Libplanet/Blockchain/Policies/IBlockPolicy.cs
11:    /// for a <see cref="Block{T}"/> to be mined.
39:        /// for a new <see cref="Block{T}"/> to be mined
43:        /// followed by a new <see cref="Block{T}"/> to be mined.</param>
45:        /// for a new <see cref="Block{T}"/> to be mined.</returns>

Libplanet.Tests/Blockchain/BlockChainTest.cs
39:            Block<DumbAction> block = _blockChain.MineBlock(_fx.Address1);
43:            Block<DumbAction> anotherBlock = _blockChain.MineBlock(_fx.Address2);
53:            Block<DumbAction> block = _blockChain.MineBlock(_fx.Address1);
56:            Block<DumbAction> anotherBlock = _blockChain.MineBlock(_fx.Address2);
64:            _blockChain.MineBlock(_fx.Address1);
121:            var lastBlock = chain.MineBlock(_fx.Address1);
151:            chain.MineBlock(_fx.Address1);
171:            var block1 = _blockChain.MineBlock(_fx.Address1);
172:            var block2 = _blockChain.MineBlock(_fx.Address1);
173:            var block3 = _blockChain.MineBlock(_fx.Address1);
198:            var block1 = _blockChain.MineBlock(_fx.Address1);
199:            var block2 = _blockChain.MineBlock(_fx.Address1);
200:            var block3 = _blockChain.MineBlock(_fx.Address1);
212:                .Select(_ => _blockChain.MineBlock(_fx.Address1))
249:            chain.MineBlock(_fx.Address1);

Libplanet.Tests/Blocks/BlockTest.cs
47:            Block<PolymorphicAction<BaseAction>> next = MineNext(_fx.Genesis);
180:            Block<DumbAction> genesis = MineGenesis<DumbAction>();
181:            Block<DumbAction> blockIdx1 = MineNext(
215:            Block<DumbAction> blockIdx2 = MineNext(
288:            Block<DumbAction> invalidBlock = MineNext(
289:                MineGenesis<DumbAction>(),
321:            Block<DumbAction> invalidBlock = MineNext(
322:                MineGenesis<DumbAction>(),
359:            Block<PolymorphicAction<BaseAction>> invalidBlock = MineNext(
375:            var block = Block<DumbAction>.Mine(
419:                transactions: MineGenesis<DumbAction>().Transactions
449:                transactions: MineGenesis<DumbAction>().Transactions

Docs/articles/design.md
42:[daemon] runnable on miners' machines (e.g., Linux).
78:when the block is mined.  It is also known as [consensus]
81:It takes 5 to 20 seconds to mine a block.  It means that if we group four

Libplanet.Tests/Net/SwarmTest.cs
165:                        var block = chain.MineBlock(_fx1.Address1);
167:                            $"Block mined. " +
178:            var minerCanceller = new CancellationTokenSource();
179:            Task miningA = CreateMiner(a, chainA, 5000, minerCanceller.Token);
180:            Task miningB = CreateMiner(b, chainB, 8000, minerCanceller.Token);
191:                minerCanceller.Cancel();
416:            Block<DumbAction> genesis = chainA.MineBlock(_fx1.Address1);
418:            Block<DumbAction> block1 = chainA.MineBlock(_fx1.Address1);
419:            Block<DumbAction> block2 = chainA.MineBlock(_fx1.Address1);
483:            chainB.MineBlock(_fx1.Address1);
528:            chainA.MineBlock(_fx1.Address1);
572:            Block<DumbAction> genesis = chainA.MineBlock(_fx1.Address1);
578:                chainA.MineBlock(_fx1.Address1);
584:                chainB.MineBlock(_fx2.Address1);

Libplanet.Tests/Blockchain/Policies/BlockPolicyTest.cs
59:            Block<DumbAction>[] blocks = MineBlocks(
101:                    MineBlocks(
116:                    MineBlocks(
131:                    MineBlocks(
150:                    MineBlocks(
158:                    MineBlocks(
166:                    MineBlocks(
174:                    MineBlocks(
185:                    MineBlocks(
205:                    MineBlocks(
215:                    MineBlocks(
223:        private IEnumerable<Block<DumbAction>> MineBlocks(
228:            _output.WriteLine($"MineBlocks({blockArgs.Length}):");
229:            var miner = default(Address);
254:                Block<DumbAction> block = Block<DumbAction>.Mine(
257:                    miner,

🤪

@codecov

This comment has been minimized.

Copy link

commented Apr 4, 2019

Codecov Report

Merging #174 into master will decrease coverage by 3.16%.
The diff coverage is 100%.

@@           Coverage Diff            @@
##           master   #174      +/-   ##
========================================
- Coverage   87.17%    84%   -3.17%     
========================================
  Files          71     71              
  Lines        3243   3245       +2     
========================================
- Hits         2827   2726     -101     
- Misses        416    519     +103
Impacted Files Coverage Δ
Libplanet/Tx/Transaction.cs 94.83% <ø> (ø) ⬆️
Libplanet/Store/FileStore.cs 93.97% <ø> (ø) ⬆️
Libplanet/Blocks/Block.cs 97.95% <100%> (ø) ⬆️
Libplanet/Blocks/RawBlock.cs 100% <100%> (ø) ⬆️
Libplanet/Action/ActionContext.cs 100% <100%> (ø) ⬆️
Libplanet/Blockchain/BlockChain.cs 98.37% <100%> (ø) ⬆️
Libplanet/Net/IceServer.cs 0% <0%> (-100%) ⬇️
Libplanet/Net/IceServerException.cs 0% <0%> (-100%) ⬇️
Libplanet/Net/NetworkStreamProxy.cs 0% <0%> (-80.77%) ⬇️
Libplanet/Net/Swarm.cs 81.78% <0%> (-5.72%) ⬇️
@dahlia

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

@kijun Could you create a new issue suggesting to change the term “mine”/“miner”? We should discuss about that for a long run.

@earlbread

This comment has been minimized.

Copy link
Member Author

commented Apr 4, 2019

Added a commit to rename Block<T>.RewardBeneficiary to Block<T>.Miner. @dahlia @longfin

Show resolved Hide resolved CHANGES.md Outdated

@earlbread earlbread force-pushed the earlbread:add-miner-to-action-context branch from d3b8398 to 090cbed Apr 4, 2019

@dahlia

dahlia approved these changes Apr 4, 2019

@longfin

longfin approved these changes Apr 4, 2019

@earlbread earlbread merged commit c1dd2f0 into planetarium:master Apr 4, 2019

5 checks passed

Travis CI - Pull Request Build Passed
Details
WIP Ready for review
Details
changelog This contains self-describing changelog.
Details
codecov/patch 100% of diff hit (target 87.17%)
Details
codecov/project Absolute coverage decreased by -3.16% but relative coverage increased by +12.82% compared to 790fcc8
Details

@earlbread earlbread deleted the earlbread:add-miner-to-action-context branch Apr 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.