gridcoin-community / Gridcoin-Research

Gridcoin-Research
MIT License
588 stars 173 forks source link

Repetitive calls to SerializeBoincBlock in Miner #295

Closed tomasbrod closed 7 years ago

tomasbrod commented 7 years ago

While reading the part about mining (staking) I noticed that SerializeBoincBlock is called many times in different places. The result sometimes ends up in HashBoinc field of the stake transaction and is overwritten later or just deleted. SerializeBoincBlock despite it's name actually signs (something) with the beacon keys ("Signing Block for cpid..."). It is called when stake block is prepared (0.5s), before searching for the stake kernel (16s), after creating a stake and once more after successful stake. Also the miner creates a block every 0.5s just to be discarded 31 times and only on 32th try it actually attempts to search for StakeKernel.

It is really confusing. And unclear whether the HashBoinc (which is not a hash btw) is included into the Stake Kernel. Also the code jumps too much from miner.cpp to main.cpp thenwallet.cpp.

iFoggz commented 7 years ago

and DeserializeBoincBlock. seems like an interesting way to move/store a lot of information easily. https://github.com/gridcoin/Gridcoin-Research/blob/master/src/main.cpp#L8503 https://github.com/gridcoin/Gridcoin-Research/blob/master/src/main.cpp#L8577

I've seen the outputs in debug before during staking but never put too much thought into the information it has stored between the delimiter or where it exactly uses it. I learn something new everyday.

neat find

tomasbrod commented 7 years ago

seems like an interesting way to move/store a lot of information easily.

Easily? Just pass the original struct, no need to convert it to string and back. In addition to converting to/from string, which is inefficient operation on it's own, Serialize also cryptographically signs the data.

The HashBoinc (output of SerializeBoincBlock) ultimately ends up in the stake transaction to approve the reward amount and authorize you as a ovner of the cpid. Other nodes then use the hashboinc to check you are not rewarding yourself more than you are owed in research and interest. Lot of the fields from hashboinc are left empty or filled with some content and newer checked. Only some, like the cpid, beacon key and signature are checked, which adds confusion.

iFoggz commented 7 years ago

@tomasbrod "I said it seems like an interesting way to move/store a lot of information easily." I'm 20 years behind on programming and just getting back into the swing of learning stuff again as its like going from Chinese simplified to Chinese traditional. I said that because I would of never thought of using that method let alone using original struct.

I agree a more efficient method could be used. Sending of information that is not checked/used/or empty seems like a waste of bandwidth and not the best practice.

Its a great find.

denravonska commented 7 years ago

I've also questioned this behavior. @tomasbrod Do you know why it discards 31 blocks before it proceeds?

tomasbrod commented 7 years ago

Yes. Because its crappy code. The miner runs at 500ms intervals. Stake timestamp mask is 16 seconds. That is all blocks created in that 16 second period have the same outcome. And someone placed an condition there to avoid calculating rewards and transactions and kernels too often.

denravonska commented 7 years ago

@tomasbrod Where is the 16 second limit? I want to dig some to see if things are the way they are because of old PoW code.

Edit: Found it in kernel.cpp. Not PoW, hmmm.

static const int STAKE_TIMESTAMP_MASK = 15;
denravonska commented 7 years ago

Can/should we do something simple like checking the stake time before creating a block in miner.cpp or did you have something more intricate in mind?

Other cleanups we can do in that function:

tomasbrod commented 7 years ago

Thinking aloud what actions are necessary for mining. What is the correct and optimal ordering

tomasbrod commented 7 years ago

In #301 serialize is called much less often. Data is no longer signed in serialize, but separately in Miner.