search

digitalbazaar / bedrock-ledger-consensus-continuity

Web Ledger Continuity Consensus Protocol
Other
3 stars 2 forks source link

`_localPeer.generate` returns result of database operation while it appears it is supposed to just return peerId #250

Closed aljones15 closed 3 years ago

aljones15 commented 3 years ago

On initial generation of a localPeer:

https://github.com/digitalbazaar/bedrock-ledger-consensus-continuity/blob/52ac4a1b48e618df0cc649833b1033d41e32a657/lib/localPeers.js#L56-L68

localPeer.generate returns the result of _generateLocalPeer.

_generateLocalPeer returns the result of api.storage.add

https://github.com/digitalbazaar/bedrock-ledger-consensus-continuity/blob/52ac4a1b48e618df0cc649833b1033d41e32a657/lib/localPeers.js#L245-L255

api.storage.add returns the result of the mongodb operation:

https://github.com/digitalbazaar/bedrock-ledger-consensus-continuity/blob/52ac4a1b48e618df0cc649833b1033d41e32a657/lib/localPeers.js#L168-L193

This results in _localPeer.generate returning the result of a database operation in the property {peerId} when no existing localPeer exists, but if a localPeer already exists, the function errors and returns the peerId itself and not an object:

https://github.com/digitalbazaar/bedrock-ledger-consensus-continuity/blob/52ac4a1b48e618df0cc649833b1033d41e32a657/lib/localPeers.js#L257-L260

So basically _localPeers.generate has an inconsistent return type for {peerId}. Sometimes {peerId} is a string and if the localPeer creation succeeds it is an object.

dlongley commented 3 years ago

So, can we just make this one line change to fix it?:

   try {
      ({localPeer: {peerId}} = await _generateLocalPeer({ledgerNodeId}));
    } catch(e) {
dlongley commented 3 years ago

Addressed by #249.