-
Notifications
You must be signed in to change notification settings - Fork 182
Superblock Contract Forwarding #1060
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
Superblock Contract Forwarding #1060
Conversation
note: I found that |
bug found, hold on merge |
corrected the compare mistake, added locks and |
Here is my start at the same functionality. What I like about it is that it either gets the contract from the NN or via a relay but other than that the code stays the same. It is not as feature complete as @tomasbrod's implementation though. |
Your approach to hook into existing code is more straightforward and also shorter. I edited what I was more familiar with. It too does either get the contract from NN call or from relay cache. |
void QuorumResponseHook(CNode* fromNode, const std::string& neural_response); | ||
void SendResponse(CNode* fromNode, const std::string& req_hash); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no apparent reason to keep this in miner. Can it be moved to another file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it can be moved, and probably should. I put it in miner, because it was related to adding neural contracts to mined blocks. If you are talking about the declarations being in the .h file, they are there because the functions are defined in the .cpp file.
@@ -7116,6 +7116,12 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, | |||
} | |||
else if (neural_request=="neural_hash") | |||
{ | |||
if(0==neural_request_id.compare(0,13,"supercfwd.rqa")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why can't this be compared directly? In miner.cpp:686 it looks like "supercfwd.rqa" is the entire content of the request id.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for extensibility. I am re-using the request-ID field and there might be a need for a actual request ID.
std::string sBinContract; | ||
bool fEnable(false); | ||
|
||
int RequestAnyNode(const std::string& consensus_hash) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't these be in net.cpp or maybe superblock.cpp?
Hey, if you want to move the functions, you can ask me or move them yourself. I ask, because I do not know what to move where. Create a new file? |
if(req_hash==sCacheHash) | ||
{ | ||
if(fDebug10) LogPrintf("supercfwd.SendResponse: %s requested %s, sending forwarded binary contract (size %d)",fromNode->addrName,req_hash,sBinContract.length()); | ||
fromNode->PushMessage("neural", std::string("supercfwdr"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Outside of this scope, but what do you say we make these message constants later?
const std::string SUPERBLOCK_FWD_RESP("supercfwdr");
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely!
The "message type" (first string in message, saying what it is) should even be 8-bit integer constant.
Superblock Contract Forwarding
Added: - Linux nodes can now stake superblocks using forwarded contracts #1060 (@tomasbrod). Changed: - Replace interest with constant block reward #1160 (@tomasbrod). Fork is set to trigger at block 1420000. - Raise coinstake output count limit to 8 #1261 (@tomasbrod). - Port of Bitcoin hash implementation #1208 (@jamescowens). - Minor canges for the build documentation #1091 (@Lenni). - Allow sendmany to be used without an account specified #1158 (@Foggyx420). Fixed: - Fix `cpids` and `validcpids` not returning the correct data #1233 (@Foggyx420). - Fix `listsinceblock` not showing mined blocks to change addresses #501 (@Foggyx420). - Fix crash when raining using a locked wallet #1236 (@Foggyx420). - Fix invalid stake reward/fee calculation (@jamescowens). - Fix divide by zero bug in `getblockstats` RPC #1292 (@Foggyx420). - Bypass historical bad blocks on testnet #1252 (@Quezacoatl1). - Fix MacOS memorybarrier warnings #1193 (@ghost). Removed: - Remove neuralhash from the getpeerinfo and node stats #1123 (@Foggyx420). - Remove obsolete NN code #1121 (@Foggyx420). - Remove (lower) Mint Limiter #1212 (@tomasbrod).
…ercfwd" This reverts commit cd020a4.
This all works only when superblock is needed and when not disabled by
-supercfwd=0
.When node discovers a peer that has with neural hash that is currently popular, it requests neural contract from that peer. When contract is received it is checked and saved in memory. When this node stakes, and it's local NN contract is empty, it uses the forwarded contract to stake superblock.
Netcmd
neural/neural_hash
is used to discover peers with contract. This netcmd is supported by current nodes (without this pr). In this PR, special flag and popular neural hash is appended. This allows nodes with this PR to send us contract with requested hash, if they have it, compressed in the same response. The contract is sent inneural/supercfwdr
netcmd to avoid possible NN confusion possibly caused by forwarded contract. Otherwise (without this PR), peers just send their local neural hash, and if it matches consensus, node requests the contract usingneural/quorum
netcmd. To this commandquorum_nresp
is generally generated, with the contract uncompressed.When node receives a valid contract with hash in consensus, apart from saving the contract (compressed), it also notifies its peers, so they can possibly request the contract, if they want. The notification is done with
neural/neural_hash
netcmd to nodes that are Participating but do not have a NN. The notification is not done when-listen=0
, beacause users with listen disabled usually want to save bandwidth.The Super-majority vote counting mechanism is left still broken, because changing it would be hard-fork.
And as always, I spent too much on this even moreso this is only temporary solution and very far from ideal.