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

Fix integer overflow with displayed nonce #1297

Merged
merged 1 commit into from
Oct 24, 2018
Merged

Fix integer overflow with displayed nonce #1297

merged 1 commit into from
Oct 24, 2018

Conversation

personthingman2
Copy link
Contributor

This fixes an integer overflow where many of the early PoW blocks nonces are displayed as negative.

For example, the nonce for testnet block number 2 is currently displayed as -238944000 but with the fix it is displayed as 4056023296.

Note this is only a display issue affecting getblock and getblockbynumber

@iFoggz
Copy link
Member

iFoggz commented Sep 13, 2018

univalue should support uint64_t so the conversion to int to begin with should of not been necessary. . test without the (uint64_t) and see if it u get the correct result. nNonce is a unsigned int as well

@denravonska
Copy link
Member

I think it should not be cast at all. The nonce is an unsigned int which should be handled by univalue.

@iFoggz
Copy link
Member

iFoggz commented Sep 13, 2018

i've done testing and UniValue still does not support unsigned int alone so leave it at uint64_t will make an issue for someone to take on as i've noticed quite a few areas we no longer need casting with latest UniValue

@iFoggz iFoggz mentioned this pull request Sep 13, 2018
4 tasks
@tomasbrod
Copy link
Member

I second that this should be handled by univalue, but it is not.

@iFoggz
Copy link
Member

iFoggz commented Sep 17, 2018

we maybe should consider putting a PR up to UniValue repo.

https://github.com/jgarzik/univalue the version is up to 1.0.4 currently but not seeing anything that addresses this kind of issue. bitcoin still on 1.0.3 and we are as well

@denravonska denravonska added this to the Camilla milestone Sep 27, 2018
@denravonska denravonska merged commit 4354452 into gridcoin-community:development Oct 24, 2018
denravonska added a commit that referenced this pull request Apr 3, 2019
Added:
 - Add `rainbymagnitude` RPC command #1235 (@Foggyx420).
 - Add stake splitting and side staking #1265 (@jamescowens).
 - Detect and block Windows shutdown so wallet can exit cleanly #1309
   (@jamescowens).
 - Add message support to sendfrom and sendtoaddress #1400 (@denravonska).

Changed:
 - Configuration options are now case insensitive #294 (@Foggyx420).
 - Update command in beaconstatus help message #1312 (@chrstphrchvz).
 - Improve synchronization speeds:
   - Refactor superblock pack/unpack #1194 (@denravonska).
   - Optimize neuralsecurity calculations #1255 (@denravonska).
   - Reduce hash calculations when checking blocks #1206 (@denravonska).
 - Make display of private key in beaconstatus OPT-IN only #1275 (@Foggyx420).
 - Store Beacon keys in Wallet #1088 (@tomasbrod).
 - Use default colors for pie chart #1333 (@chrstphrchvz).
 - Show hand cursor when hovering clickable labels #1332 (@chrstphrchvz).
 - Update README.md #1337 (@Peppernrino).
 - Fix integer overflow with displayed nonce #1297 (@personthingman2).
 - Improve application cache performance #1317 (@denravonska).
 - Improve reorg speeds #1263 (@denravonska).
 - Update Polish translation #1375 (@michalkania).

Fixed:
 - Remove expired polls from overview page #1250 (@personthingman2).
 - Fix plural text on block age #1304 (@scribblemaniac).
 - Fix researcher staking issue if your chain head was staked by you,
   #1299 (@denravonska).
 - Fix incorrect address to grcpool node #1314 (@wilkart).
 - Do not replace underscores by spaces in Qt Poll URLs #1327 (@tomasbrod).
 - Fix scraper SSL issues #1330 (@Foggyx420).

Removed:
 - Remove or merged several RPC commands #1228 (@Foggyx420):
    - `newburnaddress`, removed.
    - `burn2`: Removed.
    - `cpid`: Merged into `projects`.
    - `mymagnitude`: Merged into `magnitude`.
    - `rsa`: Removed, use `magnitude`.
    - `rsaweight`: Removed, use `magnitude`.
    - `proveownership`: Removed.
    - `encrypt`: Removed.
 - Remove obsolete POW fields from RPC responses #1358 (@jamescowens).
 - Remove obsolete netsoft fields for slight RAM requirement reduction
   #1336 (@denravonska).
 - Remove unused attachment functionality #1345 (@denravonska).
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

Successfully merging this pull request may close these issues.

None yet

4 participants