ProcessGetData, dealing with blocks + txs in batch - loop issue

[sidhujag]

https://github.com/dashpay/dash/blob/master/src/net_processing.cpp#L1278 this should probably be continue and not break.. if you get multiple getdata requests some block some txs you still want to process the txs.

[UdjinM6]

That's by design. Processing getdata for blocks can be quite expensive, so we only do this once per call. It works like this: process all known invs while it's not (some kind of) a block, process that block, drop all processed invs from vRecvGetData https://github.com/dashpay/dash/blob/master/src/net_processing.cpp#L1433. The rest of vRecvGetData is going to be processed on another call, up to another block, and so on.

[sidhujag]

it will break right now soon as it gets block so it will process maybe some INV's not blocks and soon as it gets to a block it will stop and continue on next loop.. is this to optimize tx INV's over blocks?

[UdjinM6]

As far as I understand it It's basically to avoid processing multiple heavy operations in a row - many other objects are cached and/or small, blocks on the other hand can be few MiBs large, require disk i/o and might also require chain activation. So in order to keep our communication with peers live we have to process data in batches here.

[sidhujag]

ok cool, maybe it should be made more clear or better there is a way to make it more consistent with upstream bitcoin by not even going in there if your a block you can see how I did it here: https://github.com/syscoin/syscoin/blob/dev-4.x-evo/src/net_processing.cpp#L1782

Side note: you also have to deal with m_tx_relay being nullptr meaning block only relay mode. Because BTC assumes if its not a block its a tx, you need to handle the case where its not block or tx but MN types, for this I made IsMnType() and you can see where I used it.

[PastaPastaPasta]

I haven't looked into it, but I'm going to guess that is a bitcoin PR that does what your describing that we haven't backported yet.

[sidhujag]

The tx_relay thing maybe, but might be better design to check mntype upfront and where notfound is referenced as it is assumed in bitcoin that if its not a block its a tx but in our case we have other types to deal with.

In general its probably a cleaner logic flow