First set of changes after 1-on-1 review with CG

1 files changed, 77 insertions, 83 deletions

diff --git a/draft-schanzen-r5n.xml b/draft-schanzen-r5n.xml
index 14487b2..9e7dd1a 100644
--- a/draft-schanzen-r5n.xml
+++ b/draft-schanzen-r5n.xml
@@ -445,7 +445,7 @@ Connectivity | |Underlay|  |Underlay|
           When the connection attempt is successful, information on the new
           peer is offered through the <tt>PEER_CONNECTED</tt> signal.
         </dd>
-        <!-- FIXME: Not used anywhere in draft -->
+             If PEER_CONNECT and put in routing table, then HOLD -->
         <dt>
           <tt>HOLD(P)</tt>
         </dt>
@@ -455,7 +455,6 @@ Connectivity | |Underlay|  |Underlay|
           of active connections.  With this function the DHT can indicate to the
           underlay which connections should preferably be preserved.
         </dd>
-        <!-- FIXME: Not used anywhere in draft -->
         <dt>
           <tt>DROP(P)</tt>
         </dt>
@@ -564,6 +563,8 @@ Connectivity | |Underlay|  |Underlay|
         Upon receiving a connection notification from the Underlay through
         <tt>PEER_CONNECTED</tt>, information on the new neighbor
         <bcp14>MUST</bcp14> be added to the routing table.
+        Peers added to the routing table <tt>SHOULD</tt> be signalled to the
+        Underlay as important connections using <tt>HOLD</tt>.
         Similarly when a disconnect is indicated by the Underlay through
         <tt>PEER_DISCONNECTED</tt> messages for all addresses of the peer it
         <bcp14>MUST</bcp14> be removed from the routing table.
@@ -633,44 +634,43 @@ Connectivity | |Underlay|  |Underlay|
           While details on how the first connection is established <bcp14>MAY</bcp14>
           depend on the specific implementation, this <bcp14>SHOULD</bcp14> usually be done
           by an out-of-band exchange of the information from a <tt>HELLO</tt> block.
-          <!-- FIXME It is unclear why this is needed and why it is here. THis is a serialization format
-               of the HELLO block that is not used anywhere in the draft. I already moved the formati itself
-               into the appendix. -->
-          For this, section <xref target="hello_url"/> specifies a URL format for encoding HELLO
-          blocks as text strings which <bcp14>SHOULD</bcp14> be supported by implementations.
+          Section <xref target="hello_url"/> specifies a URL format for encoding HELLO
+          blocks as text strings which allow portable, human-readable, text-based serialization
+          format that can, for example, be encoded into a QR for dissemination.
+          HELLO URLs <bcp14>SHOULD</bcp14> be supported by implementations for both import and export
+          of <tt>HELLO</tt>s.
         </t>
         <t>
           <!-- FIXME REPL_LVL unclear, RF_SIZE unclear. Sections like this make me wonder how
-               we can avoid using prose to specify message creation -->
+               we can avoid using prose to specify message creation.
+               REPL_LVL: For hello it is 4 => RECOMMENDED to 4
+               RF_SIZE: Specified in HELLO block as connected peers = |E|.
+               RF: All peers I am already connected to should be included in the RF BF
+               XQUERY: => nil
+               Flags? FindApproximate
+          -->
           To discover peers for its routing table, a peer will initiate <tt>GetMessage</tt> requests
           <xref target="p2p_get"/> asking for blocks of type  <tt>HELLO</tt>  using its own peer address as
-          <tt>QUERY_HASH</tt>, filtering all of its neighbors via the Bloom filter described
-          in <xref target="result_filter"/> and <xref target="hello_block"/>.
+          <tt>QUERY_HASH</tt>.
+          The <tt>PEER_BF</tt> is initialized and set using the peers own peer address as well as the addresses
+          of all currently connected peers.
           These requests <bcp14>MUST</bcp14> use the <tt>FindApproximate</tt> and <tt>DemultiplexEverywhere</tt>
           flags. <tt>FindApproximate</tt> will ensure that other peers will reply
           with keys they merely consider close-enough, while <tt>DemultiplexEverywhere</tt>
           will cause each peer on the path to respond, which is likely to yield
-           <tt>HELLO</tt> s of peers that are useful somewhere in the routing table.
+          <tt>HELLO</tt> s of peers that are useful somewhere in the routing table.
+          The <tt>RECOMMENDED</tt> replication level set in the <tt>REPL_LVL</tt> field is 4.
+          The size and format of the result filter is specified in <xref target="hello_block"/>.
+          The <tt>XQUERY</tt> is empty.
         </t>
         <t>
           In order to facilitate the above,
           the Underlay is expected to provide the implementation with one or more
           addresses signalled through <tt>ADDRESS_ADDED</tt>. Zero addresses <bcp14>MAY</bcp14> be
           provided if a peer can only establish outgoing connections and is otherwise unreachable.
-          <!-- Periodically? sometimes? on change? -->
-          An implementation <bcp14>MUST</bcp14> advertise its addresses by:
-        </t>
-        <!-- FIXME both things here are grossly underdefined for a MUST:
-             Message flags, BF, REPL_LVL etc
-             -->
-        <ul>
-          <li>Initiating <tt>HelloMessage</tt>s to the peers in its routing table (<xref target="p2p_hello_wire"/>).</li>
-          <li>Initiating <tt>PutMessage</tt>s containing <tt>HELLO</tt> blocks (<xref target="hello_block"/>).</li>
-        </ul>
-        <t>
-          <!-- FIXME This here sais that this is a mandatory functionality. in HelloMessage it
-               sais RECOMMENDED
-               FIXME: Expiration time of messages unclear -->
+          An implementation <bcp14>MUST</bcp14> advertise its addresses periodically to its neighbors through <tt>HelloMessage</tt>s.
+          The advertisement interval and expiration should be configurable or chosen at the discretion of the implementation based
+          on external factors such as DHCP leases.
           The specific frequency of advertisements <bcp14>MAY</bcp14> depend on available bandwidth,
           the set of already connected neighbors,  the workload of the system and other factors which are at the discretion of
           the developer, but <bcp14>SHOULD</bcp14> be a fraction of the expiration period.
@@ -683,11 +683,13 @@ Connectivity | |Underlay|  |Underlay|
       <section anchor="routing_bloomfilter">
         <name>Peer Bloom Filter</name>
         <t>
+          <!-- FIXME HArdcoded to 128 bytes = L/8 -->
           As DHT <tt>GetMessage</tt>s and <tt>PutMessage</tt>s traverse a random path through the network for the
           first N hops, it is essential that routing loops are avoided.
-          In R<sup>5</sup>N, a Bloom filter is part of the routing metadata in
-          messages in order to prevent circular routes.
-          The Bloom filter is updates at each hop with the hops
+          This peer Bloom filter is constant in size at <tt>L=1024</tt> buckets (128 bytes) and
+          <tt>k=16</tt> buckets per element.
+          The peer Bloom filter is part of the routing metadata in
+          messages in order to prevent circular routes and is updated at each hop with the hops
           peer identity.
           For the next hop selection in both the random and the deterministic
           case, any peer which is in the Bloom filter for the respective message
@@ -701,10 +703,8 @@ Connectivity | |Underlay|  |Underlay|
           traversed peers when searching for the next hops in the routing table.
         </t>
         <t>
-          The peer Bloom filter follows the definition in <xref target="bloom_filters"/>
-          and consists of L=1024 buckets.
+          The peer Bloom filter follows the definition in <xref target="bloom_filters"/>.
           The set of elements <tt>E</tt> consists of of all possible 256-bit peer IDs.
-          The number of buckets per element <tt>e</tt> is <tt>k=16</tt>.
           The mapping function <tt>M</tt> is defined as follows:
         </t>
         <t>
@@ -808,18 +808,15 @@ BEGIN
   return 1 + (RM1 / (L2NSE + RM1 * HOPCOUNT))
 ]]></artwork>
             </figure>
-            <!-- FIXME: WTF? So much wrong here.
-                 1. Why is this not part of code if necessary?
-                 2. Why is this necessary?
-                 3. Why probabillistic?
-
-                 Implementations are bound to get this wrong. -->
             <t>
               The above calculation may yield values that are
               not discrete. Hence, the result <bcp14>MUST</bcp14> be
               rounded probabilistically to the nearest
               discrete value, using the fraction
               as the probability for rounding up.
+              This probabillistic rounding is necessary to achieve
+              the statistically expected value of the replication
+              level and average number of peers a message is forwarded to.
             </t>
           </dd>
         </dl>
@@ -931,8 +928,6 @@ BEGIN
             Flags is a 16-bit vector representing binary options.
             Each flag is represented by a bit in the field starting from 0 as
             the rightmost bit to 15 as the leftmost bit.
-            <!--FIXME: Actually, we set those bits and then store the resulting
-                value in NBO...-->
           </t>
           <dl>
           <dt>0: DemultiplexEverywhere</dt>
@@ -1218,9 +1213,6 @@ BEGIN
           addresses through <tt>ADDRESS_ADDED</tt> and <tt>ADDRESS_DELETED</tt>
           it <bcp14>MAY</bcp14> disseminate those changes to neighbors using
           <tt>HelloMessage</tt>s.
-          <!-- FIXME: Are hello messages should or must?
-               Only recommended not, if so, what if that is not done?
-          -->
           Initiation of <tt>HelloMessages</tt> by the implementation itself is <bcp14>RECOMMENDED</bcp14>.
           <tt>HelloMessage</tt>s are used to inform neighbors of
           a peer about the sender's available addresses. The
@@ -1312,14 +1304,11 @@ BEGIN
             <li>
               The <tt>HELLO</tt> information is cached in the routing table until it expires,
               the peer is removed from the routing table, or the information is replaced by another message from the peer.
-              <!-- FIXME same problem as for <tt>HELLO</tt> blocks: We do not need to TRY_CONNECT here because we already have
-                   a connection, obviously. But we may want to TRY_CONNECT on the new addresses? Adding for now -->
               The implementation <bcp14>SHOULD</bcp14> instruct the Underlay to connect to all now available addresses
               using <tt>TRY_CONNECT</tt> in order to make the underlay aware of alternative addresses for this connection and
               to maintain optimal connectivity.
             </li>
             <li>
-              <!-- FIXME: We need a statement like this here. Should they? Can they? What if they are (not)? -->
               Received <tt>HelloMessages</tt> <bcp14>MUST NOT</bcp14> be forwarded.
             </li>
           </ol>
@@ -1418,8 +1407,8 @@ BEGIN
             <dt>PEER_BF</dt>
             <dd>
               A peer Bloom filter to stop circular routes (see <xref target="routing_bloomfilter"/>).
-              Set by the initiator to include itself. <!-- FIXME: Is that so? -->
-              Modified by processing peers to include their own peer ID.
+              Set by the initiator to contain the local peer and all neighbors it is forwarded to.
+              Modified by processing peers to include their own peer ID using <tt>BF-SET</tt>.
             </dd>
             <dt>BLOCK_KEY</dt>
             <dd>
@@ -1505,15 +1494,6 @@ BEGIN
               The peer address of the sender peer <tt>P</tt> <bcp14>SHOULD</bcp14> be in <tt>PEER_BF</tt>.
               If not, the implementation <bcp14>MAY</bcp14> log an error, but <bcp14>MUST</bcp14> continue.
             </li>
-            <!--<li>
-                   FIXME: I do not know what is going on here. "local peer address"
-                   is certainly wrong.
-                   But then, looking at the PathElement description, so wold be local peer ID.
-                   Also, no signatures are ever created in this processing
-              If the <tt>RecordRoute</tt> flag is set in FLAGS,
-              the local peer address <bcp14>MUST</bcp14> be appended to the <tt>PUTPATH</tt>
-              of the message.  
-            </li>-->
             <li>
               If the <tt>RecordRoute</tt> flag is not set, the <tt>PATH_LEN</tt>
               <bcp14>MUST</bcp14> be set to zero.
@@ -1531,17 +1511,16 @@ BEGIN
               If the local peer is the closest peer
               (cf. <tt>IsClosestPeer(SELF, BLOCK_KEY, PeerFilter)</tt>) or the <tt>DemultiplexEverywhere</tt>
               flag ist set, the message <bcp14>MUST</bcp14>
-              be stored locally in the block storage.
+              be stored locally in the block storage if possible.
             </li>
             <li>
-              <!-- FIXME: Discuss: Changed from MUST to SHOULD - MSC -->
               If the <tt>BTYPE</tt> of the message indicates a <tt>HELLO</tt> block, the
-              peer <bcp14>SHOULD</bcp14> be considered for the local routing
+              peer <bcp14>MUST</bcp14> be considered for the local routing
               table by using the peer address in <tt>BLOCK_KEY</tt>.
-              An implementation <bcp14>MAY</bcp14> choose to ignore the <tt>HELLO</tt>, for example
-              because the routing table or the respective k-bucket is already full.
-              If the peer is a suitable candidate for insertion, the local peer <bcp14>MUST</bcp14> try to establish a connection
-              to the peer indicated in the <tt>HELLO</tt> block using the address information
+              If the peer is not either already connected or the respective k-bucket is
+              not already full the peer <bcp14>MUST</bcp14> try to establish a
+              connection to the peer indicated in the <tt>HELLO</tt> block using
+              the address information
               from the <tt>HELLO</tt> block and the Underlay function <tt>TRY_CONNECT</tt>.
               The implementation <bcp14>MUST</bcp14> instruct the Underlay to connect to all provided addresses
               using <tt>TRY_CONNECT</tt> in order to make the underlay aware of multiple addresses for this connection.
@@ -1560,12 +1539,13 @@ BEGIN
               For each selected peer with peer address <tt>P</tt> a dedicated <tt>PutMessage_P</tt>
               is created containing the original (and where applicable already updated) fields
               of the received <tt>PutMessage</tt>.
-              In each message the local peer address <bcp14>MUST</bcp14> be added to the
+              In each message the all selected addresses and the local peer <bcp14>MUST</bcp14> be added to the
               <tt>PEER_BF</tt> and the <tt>HOPCOUNT</tt> is incremented by 1.
-              If the <tt>RecordRoute</tt> flag is set, a new Path Element is created and added
-              to the <tt>PUTPATH</tt> fields and the <tt>PATH_LEN</tt> field is incremented by 1.
-              The successor in the new Path Element is the recipient peer <tt>P</tt> of the <tt>PutMessageP</tt>.
-              Finally, the messages are sent using <tt>SEND(P, PutMessageP)</tt> each recipient.
+              If the <tt>RecordRoute</tt> flag is set, a new Path Element is created using the
+              predecessor peer ID and the signature of the current peer.
+              The Path Element is added to the <tt>PUTPATH</tt> fields and the <tt>PATH_LEN</tt> field is incremented by 1.
+              When creating the Path Element signature, the successor must be set to the recipient peer <tt>P</tt> of the <tt>PutMessageP</tt>.
+              The successor in the new Path Element is the recipient peer <tt>P</tt> of               Finally, the messages are sent using <tt>SEND(P, PutMessageP)</tt> each recipient.
             </li>
           </ol>
         </section>
@@ -1638,13 +1618,13 @@ BEGIN
             <dd>
               is a 16-bit number indicating the length of the
               result filter RESULT_FILTER.
-              Set by the initiator. Read-only. <!--FIXME is that so? -->
+              Set by the initiator. Read-only.
               In network byte order.
             </dd>
             <dt>PEER_BF</dt>
             <dd>
               A peer Bloom filter to stop circular routes (see <xref target="routing_bloomfilter"/>).
-              Set by the initiator to include itself. <!-- FIXME Is that so? -->
+              Set by the initiator to include itself and all connected neighbors in the routing table.
               Modified by processing peers to include their own peer address.
             </dd>
             <dt>QUERY_HASH</dt>
@@ -1664,7 +1644,7 @@ BEGIN
             <dt>XQUERY</dt>
             <dd>
               the variable-length extended query. Optional.
-              Set by the initiator.  <!-- FIXME: Modified by processing peers? -->
+              Set by the initiator. Read-only.
             </dd>
           </dl>
         </section>
@@ -1843,7 +1823,12 @@ BEGIN
             <dt>FLAGS</dt>
             <dd>
               is a 16-bit vector with binary options (see <xref target="route_flags"/>).
-              Set by the initiator. <!-- FIXME to what? -->
+              Set by the initiator. <!-- FIXME to what? => Copied from GET?
+              The code currently just sets the recorded PUT flags / overrides GET
+              What should happen?
+              Currently in case of HELLO => flags cleared.
+              HELLO only FindApprox
+              Application preserve flags from PUT-->
             </dd>
             <dt>PUTPATH_L</dt>
             <dd>
@@ -1966,7 +1951,8 @@ BEGIN
               does not have to match the <tt>QUERY_HASH</tt>.
             </li>
             <li>
-              <!-- FIXME: Discuss: Changed from MUST to SHOULD - MSC -->
+              <!-- FIXME: Discuss: Changed from MUST to SHOULD --
+                   same change as above  - MSC -->
               If the <tt>BTYPE</tt> of the message indicates a <tt>HELLO</tt> block, the
               peer <bcp14>SHOULD</bcp14> be considered for the local routing
               table by using the peer address computed from the block using<tt>DeriveBlockKey</tt>.
@@ -2031,7 +2017,9 @@ BEGIN
             <li>
               <!-- FIXME: If that is the case, then this also needs to be in the
                    processing of GetMessage: I think we shoul dmove this to some
-                   performance considerations instead. -->
+                   performance considerations instead.
+                   => same formulation as for PUT to store if possible (MUST)
+                   usr 1-to-1-->
               Finally, the implementation <bcp14>MAY</bcp14> choose to
               cache data from <tt>ResultMessage</tt>s.
             </li>
@@ -2304,7 +2292,10 @@ BEGIN
           <dd>
           <t>
             <!-- FIXME: I do not understand this. Isn't the element set E known
-                 for <tt>HELLO</tt> blocks? Isn't it H_ADDRS XOR MUTATOR? -->
+                 for <tt>HELLO</tt> blocks? Isn't it H_ADDRS XOR MUTATOR?
+                 E => conected peers in routing table
+                 K => is 16
+            -->
             The RESULT_FILTER for <tt>HELLO</tt> blocks is implemented using a
             Bloom filter following the definition from <xref target="bloom_filters"/>
             and consists of a variable number of buckets <tt>L</tt>.
@@ -2312,7 +2303,10 @@ BEGIN
             initiator.
             <!-- FIXME: Minimum space for used for what? There is no example given anywhere in the
                  spec how this becomes relevant. Again, this is not some abstract block,
-                 but specifically a <tt>HELLO</tt> (see above). -->
+                 but specifically a <tt>HELLO</tt> (see above).
+                 E is the number of currently connected peers (neighbors)!
+                 E is no really ever 0 so the following sentence should be amended.
+            -->
             If <tt>|E|</tt> is zero, the size <tt>L</tt> is set to 32 bits to provide some minimum
             space (to be used by peers when forwarding the request after
             returning local results).
@@ -2544,7 +2538,9 @@ BEGIN
           control such as <xref target="RFC6940"/> which, like R<sup>5</sup>N, provides
           a peer-to-peer (P2P) signaling protocol with extensible routing and topology
           mechanisms.
-          <!-- FIXME: Can we give examples here? -->
+          <!-- FIXME: Can we give examples here?
+               Centralization DNS roots fixed through GNS.
+          -->
           Some decentralized applications require a more open system that
           enables ad-hoc participation and other means to prevent common attacks
           on P2P overlays.
@@ -2605,7 +2601,9 @@ BEGIN
         GANA created the registry as follows:
       </t>
       <!-- FIXME changed GNS Reference to This.I-D because we either need to define it here
-           or in the GNS RFC. And I think here is better or in a separate document -MSC -->
+           or in the GNS RFC. And I think here is better or in a separate document
+           => not in here. Use separate document for NAMERECORD draft.
+           -MSC -->
       <figure anchor="figure_btypenums" title="The Block Type Registry.">
         <artwork name="" type="" align="left" alt=""><![CDATA[
 Number| Name           | References | Description
@@ -3001,12 +2999,8 @@ maybe generate proper test vector.
           is reachable via "udp" at 127.0.0.1 on port 2086 until
           1647134480 seconds after the Epoch.  Note that "udp"
           here is underspecified and just used as a simple example.
-          <!-- FIXME: Must be supported by which underlay?
-               This does not make sense. I may be able to generate
-               addr-names that my underlay supports, but there is not
-               way to guarantee that all underlays support it. -->
           In practice, the key (addr-name)
-          <bcp14>MUST</bcp14> refer to a scheme supported by a
+          refers to a scheme supported by a
           DHT Underlay.
         </t>
         <t>