omnilaboratory / obd

OmniBOLT daemon, a golang implementation of OmniBOLT spec, the smart assets lightning network.
MIT License
213 stars 21 forks source link

Tracker-OBD grpc communication module refactored #53

Open wxf4150 opened 2 years ago

wxf4150 commented 2 years ago

issues:

  1. service-interface :fixed old service-interfce:

    // old service/NodeAccountService api
    Login(obdClient *ObdNode, msgData string) (retData interface{}, err error)
    Logout(obdClient *ObdNode) (err error)
    UserLogin(obdClient *ObdNode, msgData string) (retData interface{}, err error)
    UserLogout(obdClient *ObdNode, msgData string) (err error)
    UpdateUserInfo(obdP2pNodeId, obdClientId, userId string) (retData interface{}, err error)
    UpdateUsers(obdClient *ObdNode, msgData string) (err error)
    GetNodeInfoByP2pAddress(context *gin.Context)
    GetUserState(context *gin.Context)
    GetUserP2pNodeId(context *gin.Context)
    GetAllUsers(context *gin.Context)
    GetAllObdNodes(context *gin.Context)
    
    // service/channel api:
    func (manager *channelManager) updateChannelInfo(obdP2pNodeId string, msgData string) (err error)
    func (manager *channelManager) GetChannelState(context *gin.Context)
    func (manager *channelManager) GetChannels(context *gin.Context)
    
    // service/htlc api:
    func (manager *htlcManager) getPath(msgData string) (path interface{}, err error)
    func (manager *htlcManager) updateHtlcInfo( msgData string) (err error)
    func (manager *htlcManager) GetHtlcCurrState(context *gin.Context)

    The old intrerfaces: have no document; input/output parameters are week types; mixed gin-handler and data-api service

New service definition applies protobuf. Details are in tracker/tkrpc/info-tracker.proto

New interfaces:

service InfoTracker {
rpc HeartBeat (stream UpdateNodeInfoReq) returns (EmptyRes);
//map to old func Logout Login
rpc UpdateNodeInfo (UpdateNodeInfoReq) returns (EmptyRes);
//map to old func userLogout userLogin
rpc UpdateUserInfo (UpdateUserInfoReq) returns (EmptyRes);
//map to old func updateUsers
rpc UpdateUserInfos (UpdateUserInfosReq) returns (EmptyRes);
//map to old func: GetUserState , GetUserP2pNodeId
//Request:SetUserInfoReq{user_id,node_Id}
//old GetUserP2pNodeId use request:SetUserInfoReq{user_id} ;  may not work when user login on multi node
rpc GetUserInfo (UpdateUserInfoReq) returns (UserInfo);
//Map to old function GetAllUsers
rpc GetUserInfos (ListReq) returns (UserInfosRes);
//map old function GetAllObdNodes
rpc GetNodes (ListReq) returns (NodeInfosRes);
// map to old ChannelService.updateChannelInfo

rpc UpdateChannelInfo(ChannelInfo)returns(ChannelInfo);
rpc UpdateChannelInfos(UpdateChannelInfosReq)returns(EmptyRes);
//map to old ChannelService.GetChannelState
rpc GetChannelInfo(SimpleFilter)returns(ChannelInfo);
rpc GetChannels(ListReq)returns (NodeInfosRes);

// map old function updateHtlcInfo
rpc UpdateHtlcInfo(HtlcInfo)returns (HtlcInfo);
//map old function getPath
rpc HtlcGetPath(HtlcGetPathReq)returns (HtlcGetPathRes);
//map old function GetHtlcCurrState
rpc GetHtlcInfo(GetHtlcInfoReq)returns (HtlcInfo);
}

//obd-user-info
message UserInfo {
    int32 id = 1;
    //user_peer_id
    string  user_id = 2;
    //node_peer_id
    string  node_id = 3;
    // 1 online 2 offline
    int32 is_online = 4;
    string access_ip = 5;
    //unix timestamp in seconds ; google.protobuf.Timestamp will more better,but sql database not support protobuf.Timestamp.
    int64  updated_at = 6;
    int64  created_at = 7;
}
...
...

Strong-typed data structure. Added/updated some comments as document in the proto definition file, and the document is displayed synchronously in proto genrated go-files, and will be automatically integrated in swagger document too, or obd js-sdk which invokes the rpc-service.

  1. (fixed) one user connects to multiple obd-nodes ( tracker? ) which call getUserP2pNodeId from remote trackers. This will not work. Fixed: now an obd only connects one tracker when sync/query data.

  2. (fixed) Tracker side updateUsers does not set userinfo.ObdP2pNodeId. userInfo should sync this field.

  3. (fixed) omnibolt-report-final.pdf refer: OBD uses not maintained dependencies ( ile-rotatelogs, asdine/storm, etc.). Fixed: storm is delete, tracker uses gorm database model now.

  4. (fixed) Tracker/dao/pojo.go UserInfo.ObdNodeId fieldname hard to understand.

  5. (fixed) func (manager htlcManager) getPath(obdClient ObdNode, msgData string) (path interface{}, err error) at tracker/service/htlc_service.go lines 40.
    the retured paramter path may be a string or a map; When consume returned value, you should use reflect to detect type of the return value at lightclient/connect_tracker.go lines 107, which is a bad desgin https://github.com/omnilaboratory/obd/blob/58293151d0122daf331a518cea5105bd2e619374/lightclient/connect_tracker.go#L98-L114

  6. (fixed) userState mananger, userOnlineOfOtherObdMap .fixed
    old code
    https://github.com/omnilaboratory/obd/blob/58293151d0122daf331a518cea5105bd2e619374/tracker/service/p2p_service.go#L213-L245 it's hard to understand the the meaning of the var-params at line 225 from the context, you must seek the message source to understand the exact meaning of this variable, which is hard to maintain the source code. Concurrent map access is a problom too, which is now discarded.

the new code version :

func (s *ImpInfoServer) UpdateUserInfo(ctx context.Context, req *tkrpc.UpdateUserInfoReq) (*tkrpc.EmptyRes, error) {
    uinfo := new(tkrpc.UserInfo)
    err := Orm.FirstOrCreate(uinfo, tkrpc.UserInfo{UserId: req.UserId, NodeId: req.NodeId}).Assign(tkrpc.UserInfo{IsOnline: req.IsOnline}).Error
    return &tkrpc.EmptyRes{}, err
}
  1. (fixed) Two duplicated function: tracker/service/p2p_service.go sendChannelUnlockInfoToObd SendChannelLockInfoToObd
    https://github.com/omnilaboratory/obd/blob/58293151d0122daf331a518cea5105bd2e619374/tracker/service/p2p_service.go#L266-L300 https://github.com/omnilaboratory/obd/blob/58293151d0122daf331a518cea5105bd2e619374/tracker/service/p2p_service.go#L349-L385

  2. (to to)channelInfo will be updated on both sides of tracker and obd-server; When an obd starts, obd's channels-Info data will cover the data from the tracker, the tracker's update will lost. The sync mechanism between obd and tracker causes data error.

  3. (to to) channelInfo's fields on tracker are different to the obd.
    When obd commits channelInfo to a tracker, now we have the code to convert:

    //infoRequest is the data submit to tracker
    infoRequest.PeerIdA = channelInfo.PeerIdA
    infoRequest.PeerIdB = channelInfo.PeerIdB
    
    infoRequest.IsAlice = false
    if commitmentTx.Id > 0 {
        if commitmentTx.Owner == channelInfo.PeerIdA {
            infoRequest.IsAlice = true
            infoRequest.AmountA = commitmentTx.AmountToRSMC
            infoRequest.AmountB = commitmentTx.AmountToCounterparty
        } else {
            infoRequest.AmountB = commitmentTx.AmountToRSMC
            infoRequest.AmountA = commitmentTx.AmountToCounterparty
        }
    }

    We should modify channelInfo's fields to be the same as the tracker. It will be easy when updates the sync mode between the obd and tracker.

channelInfo.IsAlice should be a function, it's a computed field. For example:

func (c *channelInfo)IsAlice(currentUserPeerID) bool{
    return c.PeerIdA==currentUserPeerID
}
  1. (to to) func (manager *htlcManager) getPath(msgData string) (path interface{}, err error) shoud have a document: the main flow,and how to use the returned value.
    https://github.com/omnilaboratory/obd/blob/58293151d0122daf331a518cea5105bd2e619374/tracker/service/htlc_service.go#L40-L83

  2. (fixed) obd/service/user.go line 60 calls noticeTrackerUserLogin().sendMsgToTracker() . https://github.com/omnilaboratory/obd/blob/58293151d0122daf331a518cea5105bd2e619374/service/user.go#L20-L76

    • issue 1: if some err occurs ,we MUST not invoke it (L60). If it noticeTrackerUserLogin just record a audit-log, we can ignore the err and invoke it.
    • issue 2: obd/service is just a database/data operation module, it MUST not call sendMsgToTracker to send async message; obd/lightclient module is async message center, now 95% async messages are recieved and sent here, all the client-websocket-conn p2p-conn are initialized here; obd/service/htlc_tx_forward.go PayerRequestFindPath invoke "sendMsgToTracker(enum.MsgType_Tracker_GetHtlcPath_351, pathRequest)" have the same probolem.
    • now i have fixed the two issue, and obd/service.sendMsgToTracker have disbled, and below messages submit by grpc-conn now,no need to sync userInfo/channelInfo/htlcInfo to tracker with complex async message Goroutines. the old msgList:
    • MsgType_Tracker_UpdateChannelInfo_350
    • MsgType_Tracker_UserLogin_304
    • MsgType_Tracker_UserLogout_305
    • MsgType_Tracker_UpdateHtlcTxState_352
  3. (to to) may be we should design the obd-tracker info sync-mode and security first. Maybe new mechanism won't need the above work.

wxf4150 commented 2 years ago

now tracker's websocket server-api can switch to grc-server. but need more test.

neocarmack commented 2 years ago

Related dicussions have been put in the following issues:

https://github.com/omnilaboratory/obd/issues/55

https://github.com/omnilaboratory/obd/issues/56

https://github.com/omnilaboratory/obd/issues/57

https://github.com/omnilaboratory/obd/issues/58