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

Current UDHT implementation is not working as designed #422

Open
dinpd opened this issue Jan 17, 2020 · 0 comments
Open

Current UDHT implementation is not working as designed #422

dinpd opened this issue Jan 17, 2020 · 0 comments
Labels
bug Something isn't working

Comments

@dinpd
Copy link
Contributor

dinpd commented Jan 17, 2020

Reference design document:
https://github.com/graft-project/DesignDocuments/blob/master/RFCs/%5BRFC-004-COMM%5D-Communication.md

Discussion about comm options:
graft-project/DesignDocuments#1

The point of JumpList/UDHT in few words:

Each supernode maintains a randomly selected subset of global "ID -> network address" map. While processing incoming P2P broadcast message, cryptonode sees if its local supernode knows the destination IP and if yes - it forwards message to supernode, which, in turn, issues direct connection to a destination supernode and delivers a message to it.

Questions
Q1. How UDHT currently implemented in Graft actually works and how far it is from the design

A1.

  1. Supernode introduces two endpoints and one periodic call for this purpose

1.1 - periodicUpdateRedirectIds() - periodically announces (multicasts, period is stake_wallet_refresh_interval_ms from config) its encrypted node_address (pubkey, ip, port) to the randomly (?) selected (more complex than just random, see redirect.cpp, fillSubsetFromAll function) subset of active supernodes over P2P network (seems like in design document) using /cryptonode/update_redirect_ids endpoint

BUG found:
it announces local ip address (0.0.0.0.) instead of external one, but we also should handle NAT case properly

1.2 /cryptonode/update_redirect_ids - handles periodic address publishing from supernodes. updates a map (ID -> network address) stored in global context.
handler is implemented in redirect.cpp, onUpdateRedirectIds function

it does something weird (unclear) at the end:

// register ID of another supernode to redirect
// TODO: IK20200116 not clear what is it for
SupernodeRedirectIdsJsonRpcRequest oreq;
oreq.params.id = pubkey;
oreq.params.my_id = my_pubIDstr;
oreq.method = "redirect_supernode_id";
oreq.id = 0;

        ctx.local["oreq"] = oreq;
        return graft::Status::Again;
    } break;
    //   
    case graft::Status::Again:
    {
        SupernodeRedirectIdsJsonRpcRequest oreq = ctx.local["oreq"];
        output.load(oreq);
        output.path = "/json_rpc/rta";
        return graft::Status::Forward;
    } break;
  • calls redirect_supernode_id method on cryptonode side
    TODO: make the list persistent (keep it while process not running)

1.3 /redirect_broadcast - handles message, delivered over P2P from another supernode to be forwarded to the destination supernode which address is known to our supernode. To be forwarded via direct supernode-supernode connection.
handler is implemented in redirect.cpp, onRedirectBroadcast function

  1. Cryptonode introduces following RPC methods:

2.1 /register_supernode - called by locally connected supernode to notify cryptonode so it should forward broadcasts from P2P to local supernode.
TODO: supernode calls this periodically but it should call only once at start (this endpoint doesn't really related to UDHT, mentioned just in case)

2.2 /redirect_supernode_id - called by locally connected supernode. Right now the purpose is not completely clear but it looks like it creates a link between local supernode and remote supernode which IP address known to local supernode

@dinpd dinpd added the bug Something isn't working label Jan 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant