add mnlistdiffs

[pvrooyen]

Help needed with:

• Confirming implementation correct
• Unit tests

Currently 2 tests (maybe more) is failing here - marked with todo comments. I suspect that it is because of invalid json hex supplied in /test/data/messages.json for mnlistdiff and getmnlistdiff

[antouhou]

What kind of help with unit tests do you exactly need? You want someone to join you with writing test cases?

[pvrooyen]

@antouhou actually if we can get those 2 tests that are failing through it should be fine. As per PR description I don't know how to generate test messages.

Here are the failing tests:
https://github.com/dashevo/dashcore-p2p/blob/1d496e4ff181484a4b868664ae67b20ef68898e5/test/messages/index.js#L77
https://github.com/dashevo/dashcore-p2p/blob/1d496e4ff181484a4b868664ae67b20ef68898e5/test/messages/index.js#L82

[Cofresi]

mnlistdiff still errors after typo fix. You might want to not include that message, because it will always come from a full node as a response to getmnlistdiff and is not needed in this p2p library.
BTW if you haven't already noticed the main travis error is because you have a local link to dashcore-lib in package.json

[pvrooyen]

@Cofresi thanks for the attempted fixes. Problem is that those are generic tests so you cannot really exclude them specifically.

Can you in fact do a getmnlistdiff rpc request? If so I might rethink to keep the P2P architecture and just go for RPC.

Im getting 'Method not found'

[pvrooyen]

@antouhou @Cofresi @KamuelaFranco

I am leaving this PR here for now but not going to spend time to try and fix the errors. We are not using P2P for mnlistdiffs anymore so maybe we can just close this PR without merging?

[nmarley]

Rebased onto master. If we do end up getting this in, it would be nice to squash and possibly break out into fewer commits. But if we squash-merge (my preference), then I guess it won't matter.

[nmarley]

I have again squashed and rebased onto the latest master. I also made a couple temporary branches in case someone wanted to see this branch before my rebases.

Very original branch before rebase can be found here now: https://github.com/dashevo/dashcore-p2p/tree/feat-mndifflist-old

The rebased before squash and latest master is here: https://github.com/dashevo/dashcore-p2p/tree/TEMP-feat-mndifflist-rebased

@antouhou @Cofresi @KamuelaFranco Can you see the above comment from Pierre and verify whether we need this PR at all?

I'd love to get this PR merged or closed and branch deleted within a day or two if possible. Thanks!

[Cofresi]

@nmarley tests are fixed now. The message for mnlistdiff in messages.json had a wrong message header (command name was wrong) and payload. Replaced with a much shorter testvector for mnlistdiff. Also fixed setter and getter in messages/commands/mnlistdiff.js which caused errors.

mnlistdiff.js is not parsing the payload yet, but I still think this should be merged into dashcore-p2p to reflect the updated command list in core. We can work on the mnlistdiff payload when we will actually have a use for it. Maybe you'll want to bump the version before merging.

[nmarley]

Great, thank you!

[nmarley]

ACK

[nmarley]

Since this has now been merged, I'm going to remove the temp branches that I created earlier ^. We have these in the daily git archives if anyone needs them, just reach out.