Skip to content
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

RSAWeight and Magnitude not checked in staked blocks #345

Closed
tomasbrod opened this issue Jun 3, 2017 · 8 comments
Closed

RSAWeight and Magnitude not checked in staked blocks #345

tomasbrod opened this issue Jun 3, 2017 · 8 comments

Comments

@tomasbrod
Copy link
Member

tomasbrod commented Jun 3, 2017

Neither fields are validated in incoming blocks. RSAWeight field is currently used to give new users boost in stake weight. However it is not verified that it is indeed a new user. By modifying the source code I was able to hugely increase my weight and stake multiple times a day. MintLimiter prevents me from creating even more blocks.
Research reward is calculated from the current magnitude and magnitude at previous dpor block. The current magnitude is checked but the value in dpor block is not. I modified the code to set magnitude 30000 in my staked blocks. This further increased my stake weight. First reward was based on my legitimate mag of 18 at that time, but the next rewards were all high. I staked 7 such blocks in two days so far totaling 552 GRC. In testnet I received 10371 TestGRC. There are multiple 1GRC rewards caused by #317.
Furthermore a proof-of-work nonce is included in current stake kernel degrading the consensus algorithm to Proof of Work.

cpid/46f64d69eb8c5ee9cd24178b589af83f
I still want to support this coin. I haven't done anything wrong. I just politely asked the network to give me
some bonus and it did.
To prevent panic i would like to point out: Balance in your wallet is still and will be well safe.
Immediate action is required.

@grctest
Copy link
Contributor

grctest commented Jun 3, 2017

@gridcoin High priority issue.

@tomasbrod Additional discretion regarding security issues would be appreciated. Action regarding this cannot be immediate because a mandatory upgrade requires advance notice for exchanges/services.

Relevant issues:
#332
#317

@gridcoin
Copy link
Contributor

gridcoin commented Jun 3, 2017

Researching this issue now.

@tomasbrod
Copy link
Member Author

tomasbrod commented Jun 3, 2017

I would like if we can fix this together. I already proposed a solution in my wiki page.
I think we should completely remove rsa weight and por nonce from the stake kernel and weight calculation. Magnitude in dpor blocks must be checked too.
And base the fix on #301, it is refactored code.
After this hole is patched, we can look at the other improvements at low priority.

@gridcoin
Copy link
Contributor

gridcoin commented Jun 4, 2017

Hi @tomasbroad, I just sent you an email.

@grctest
Copy link
Contributor

grctest commented Jun 6, 2017

If we have a malformed superblock (or no superblock in a couple days), might that disrupt the verification of blocks? Would it be wise to create a failsafe in which if my client fails to stake a few times in a row due to mag inaccuracies that it falls back solely on POS then tries POR again after a set quantity of blocks?

@skcin
Copy link
Contributor

skcin commented Jun 8, 2017

@tomasbrod if the por_nonce and the RSA_WEIGHT are removed from the hash and the RSA_Weight is checked with the magnitude from the last superblock, it would be safe to keep the target weight, right?

@Quezacoatl1
Copy link
Contributor

@tomasbrod can we close this issue? I believe you have at least proposed a fix which should be tested in testnet soon, right? :)

@tomasbrod
Copy link
Member Author

Fixed in #364 .
@skcin It is not safe to ADD magnitude to utxo stake weight. as it gets multiplied by number of UTXOs.

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

No branches or pull requests

5 participants