add initial block pool #139

arnetheduck · 2019-02-27T06:25:56Z

implement in-memory block graph
store tail block in database
resolve unknown parents by syncing them from peers
introduce concept of resolved blocks and attestations - those that
follow minimal protocol rules
update state head lazily
log more stuff
shortHash -> shortLog
start 9/10 beacon nodes by default, last can be started manually
see also Pending attestation & block pool #134

tersec · 2019-02-27T20:35:20Z

tests/test_block_pool.nim

@@ -0,0 +1,89 @@
+# beacon_chain
+# Copyright (c) 2018 Status Research & Development GmbH


Here and elsewhere, what's the best thing to do with copyright years?

Omit or say "Copyright (c) 21st century, Status..."

I'm only half kidding, literally no one takes that in any way seriously, not even copyright lawyers (verified by two).

ok, let's fix this on the whole repo after merging this, in a separate PR?

Yes, this PR isn't the place to fix that.

tersec · 2019-02-27T21:56:20Z

beacon_chain/spec/validator.nim

@@ -269,6 +269,7 @@ func get_beacon_proposer_index*(state: BeaconState, slot: Slot): ValidatorIndex
  # TODO this index is invalid outside of the block state transition function
  #      because presently, `state.slot += 1` happens before this function
  #      is called - see also testutil.getNextBeaconProposerIndex
+  # TODO is the above still true? the shuffling has changed since it was written


The shuffling is still a function of seed, which is a function of epoch, so one would have to check beginning/end of epoch boundary conditions.

think what would be useful here is a unit test that shows what's safe to do in terms of state-slot vs passed-in slot

mratsim

LGTM

mratsim · 2019-02-28T10:29:19Z

tests/test_block_pool.nim

@@ -0,0 +1,89 @@
+# beacon_chain
+# Copyright (c) 2018 Status Research & Development GmbH


mratsim · 2019-02-28T10:33:10Z

beacon_chain/attestation_pool.nim

+        info "Attestation resolved",
+          slot = humaneSlotNum(attestation.data.slot),
+          shard = attestation.data.shard,
+          beaconBlockRoot = shortLog(attestation.data.beacon_block_root),


I suppose shortLog is a truncated print from chronicles?

yeah, the idea being that we can define a function with one name for all types instead of a different name for ever type

beacon_chain/attestation_pool.nim

mratsim · 2019-02-28T10:41:03Z

beacon_chain/spec/digest.nim

@@ -31,6 +31,9 @@ type
  Eth2Digest* = MDigest[32 * 8] ## `hash32` from spec
  Eth2Hash* = blake2_512 ## Context for hash function

+func shortLog*(x: Eth2Digest): string =


one name for all

zah · 2019-02-28T11:14:07Z

beacon_chain/beacon_node.nim

-
-func shortHash(x: auto): string =
-  ($x)[0..7]
+  topicfetchBlocks = "ethereum/2.1/beacon_chain/fetch"


Is the intention here that this is a temporary solution?

In Brussels, we discussed that there will be a RequestManager instance that will be responsible for querying a number of randomly selected peers for blocks and other data through the SyncProtocol (which doesn't operate in broadcast fashion and sends the requests to specific peers).

The RequestManager may have some configurable policy for trading latency for bandwidth (i.e. it will control how many redundant requests are send in parallel in case some of the peers don't respond in time). It will also keep reputation scores for the peers.

yeah, just an example to see if it works

beacon_chain/block_pool.nim

zah · 2019-02-28T12:25:45Z

tests/simulation/start.sh

@@ -59,35 +53,19 @@ USE_MULTITAIL="${USE_MULTITAIL:-no}" # make it an opt-in
 type "$MULTITAIL" &>/dev/null || USE_MULTITAIL="no"
 COMMANDS=()

-for i in $(seq 0 9); do
+for i in $(seq 0 8); do


You explained this on the call, but the audio interrupted briefly for me. Why are we skipping the last node?

the idea is to have one node to start easily separately (using ./run_node.sh 9)

* implement in-memory block graph * store tail block in database * resolve unknown parents by syncing them from peers * introduce concept of resolved blocks and attestations - those that follow minimal protocol rules * update state head lazily * log more stuff * shortHash -> shortLog * start 9/10 beacon nodes by default, last can be started manually * see also #134 * fix start.sh epoch length

Add tests for read() and write() deadlocks.

stefantalpalaru mentioned this pull request Feb 27, 2019

start.sh: use native Windows paths on Mingw #138

Merged

arnetheduck force-pushed the block-pool branch from f58c522 to 326808b Compare February 27, 2019 20:02

arnetheduck changed the title ~~[WIP] add initial block pool~~ add initial block pool Feb 27, 2019

arnetheduck requested review from mratsim, yglukhov and tersec February 27, 2019 20:04

tersec reviewed Feb 27, 2019

View reviewed changes

mratsim approved these changes Feb 28, 2019

View reviewed changes

zah approved these changes Feb 28, 2019

View reviewed changes

arnetheduck force-pushed the block-pool branch from 326808b to ddf00c9 Compare February 28, 2019 16:10

arnetheduck merged commit 125231d into master Feb 28, 2019

delete-merged-branch bot deleted the block-pool branch February 28, 2019 21:21

etan-status pushed a commit that referenced this pull request May 12, 2022

Fix deadlock for pending write() calls on transport close. (#139)

ac9b3e3

Add tests for read() and write() deadlocks.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add initial block pool #139

add initial block pool #139

arnetheduck commented Feb 27, 2019

tersec Feb 27, 2019

Swader Feb 27, 2019

mratsim Feb 28, 2019

arnetheduck Feb 28, 2019

tersec Feb 28, 2019

tersec Feb 27, 2019

arnetheduck Feb 28, 2019

tersec Feb 28, 2019

arnetheduck Feb 28, 2019

mratsim left a comment

mratsim Feb 28, 2019

mratsim Feb 28, 2019

arnetheduck Feb 28, 2019

mratsim Feb 28, 2019

arnetheduck Feb 28, 2019

zah Feb 28, 2019

arnetheduck Feb 28, 2019

zah Feb 28, 2019

arnetheduck Feb 28, 2019

		@@ -0,0 +1,89 @@
		# beacon_chain
		# Copyright (c) 2018 Status Research & Development GmbH

add initial block pool #139

add initial block pool #139

Conversation

arnetheduck commented Feb 27, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mratsim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment