Skip to content

optional nspv msg#379

Merged
ca333 merged 3 commits intodisable_nspvfrom
optional_nspv_msg
Aug 1, 2020
Merged

optional nspv msg#379
ca333 merged 3 commits intodisable_nspvfrom
optional_nspv_msg

Conversation

@ca333
Copy link
Copy Markdown

@ca333 ca333 commented Jul 27, 2020

No description provided.

@DeckerSU
Copy link
Copy Markdown

For making code better readable, i would recommend to do the same changes as following:

bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, int64_t nTimeReceived)
...
const bool fAllowProcessNSPV = GetBoolArg("-nspv_msg", false);
...
    else if (fAllowProcessNSPV && strCommand == "getnSPV")
    {
        if ( KOMODO_NSPV == 0 )//&& KOMODO_INSYNC != 0 )
        {
            std::vector<uint8_t> payload;
            vRecv >> payload;
            komodo_nSPVreq(pfrom,payload);
        }
        return(true);
    }
    else if (fAllowProcessNSPV && strCommand == "nSPV")
    {
        if ( KOMODO_NSPV_SUPERLITE )
        {
            std::vector<uint8_t> payload;
            vRecv >> payload;
            komodo_nSPVresp(pfrom,payload);
        }
        return(true);
    }
...

But variant in PR is also looks good.

@DeckerSU
Copy link
Copy Markdown

Also this #378 (comment) is good point.

Copy link
Copy Markdown

@Alrighttt Alrighttt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will approve review after #378 (comment) is implemented.

@ca333 ca333 requested a review from Alrighttt July 28, 2020 06:56
Copy link
Copy Markdown

@Alrighttt Alrighttt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This creates problems with the nspv clients trying to connect to these nodes. It does seem to be a nspv client issue(not returning ping p2p messages). The issue is not directly related to these changes, but something to keep in mind for the future as we begin to use nspv clients.

@ca333 ca333 merged commit a2e80eb into disable_nspv Aug 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants