Add Swarm.BroadcastBlocksAsync() #59
Conversation
longfin
force-pushed
the
feature/block-sync
branch
4 times, most recently
from
a49c460
to
0f1b996
Compare
| #pragma warning disable CS4014 | ||
| Task.Run(async () => await swarmA.RunAsync(chainA, 250)); | ||
| Task.Run(async () => await swarmB.RunAsync(chainB, 250)); | ||
| Task.Run(async () => await swarmC.RunAsync(chainC, 250)); |
There was a problem hiding this comment.
Shouldn't these Tasks be awaited in finally block at the latest? (As like we join all spawned threads until a program terminates?)
There was a problem hiding this comment.
We can't await these tasks because they run in infinite loop.
but we can cancel these task in finally block. I'll add it.
| { | ||
| _fx = fixture; | ||
| _fx = new FileStoreFixture(); |
There was a problem hiding this comment.
Why do you change it to use xUnit's fixture mechanism no more?
There was a problem hiding this comment.
xUnit's fixture is shared within the running cases, which causes unnecessary side effects.
| @@ -42,6 +42,8 @@ public Blockchain(IStore store) | |||
|
|
|||
| public IStore Store { get; } | |||
|
|
|||
| public Block<T> Tip => this[-1]; | |||
There was a problem hiding this comment.
Does a negative index usually mean a backward offset?
Libplanet/Blockchain.cs
Outdated
| { | ||
| HashDigest<SHA256>? current = Store.IndexBlockHash(-1); | ||
|
|
||
| while (current is HashDigest<SHA256> hash && current != point) |
There was a problem hiding this comment.
It would be more tolerant of later changes if the latter condition becomes hash != point.
Libplanet/Blockchain.cs
Outdated
|
|
||
| while (current is HashDigest<SHA256> hash && current != point) | ||
| { | ||
| HashDigest<SHA256>? next = Blocks[hash].PreviousHash; |
There was a problem hiding this comment.
It's next to access, but actually previous in the chain. Both maybe confusing, but I find “previous” is slightly more intuitive than “next” here.
Libplanet/Net/Swarm.cs
Outdated
| else | ||
| { | ||
| _logger.Information( | ||
| "Received index is lower than current chain's tip." + |
There was a problem hiding this comment.
The word “lower” here is assuming the highest is the tip and the lower is the genesis. I doesn't find it necessarily intuitive, so the term should be more concrete IMHO, like “older.”
| } | ||
| } | ||
|
|
||
| File.WriteAllBytes(_indexPath, writer.ToArray()); |
There was a problem hiding this comment.
It doesn't seem very scalable to buffer the whole index on the memory. It should be fixed in the future. It also would be good if we write a FIXME comment here.
Libplanet/Net/Swarm.cs
Outdated
| { | ||
| // We assume that the blocks are sorted in order. | ||
| Block<T> oldest = blocks.First(); | ||
| Block<T> newest = blocks.Last(); |
There was a problem hiding this comment.
It's more natural to be the latest than the newest here to me?
| Block<T> newest = blocks.Last(); | ||
| HashDigest<SHA256>? tip = blockchain.Store.IndexBlockHash(-1); | ||
|
|
||
| if (tip == null || oldest.PreviousHash == tip) |
There was a problem hiding this comment.
Does it mean the newest is expected to be the most closest to the genesis, unlike the intuition its name give us?
There was a problem hiding this comment.
No, newest is most far from genesis.
This condition means that if oldest in received blocks is next to our tip, we can connect it directly.
longfin
force-pushed
the
feature/block-sync
branch
2 times, most recently
from
7c1e723
to
c7d9243
Compare
|
In fixing #59 (comment), I realize current |
longfin
force-pushed
the
feature/block-sync
branch
from
c7d9243
to
aa55691
Compare
longfin
force-pushed
the
feature/block-sync
branch
from
aa55691
to
86884a1
Compare
Bump Libplanet version to 0.6.0
…lash-exceptions Define exceptions in HackAndSlash
This PR implements
Swarm.BroadcastBlocksAsync()for miner can broadcast mined block to known peers. the following points have also been changed to accomplish it.DeleteIndex()toIStore/FileStoreto able to give up own block indexes when other longest chain appears.Blockchain.DeleteAfter()to same reason.Blockchain.GetBlockLocator()to query block hashes to broadcasting peer.Blockchain.FindNextHash()returns result containing branch point.