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

[rpcwallet] Don't use floating point #8317

Merged
merged 1 commit into from
Jul 11, 2016
Merged

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Jul 8, 2016

Follow up of 69d605f

fun fact:
The refactor in getreceivedbyaddress actually removes the feature to detect if IsMine() was false. You can check getreceivedbyaddress == 0 and isfloat(getreceivedbyaddress)

@maflcko
Copy link
Member Author

maflcko commented Jul 8, 2016

Before:

> getreceivedbyaddress $notMineAddress
< 0
> getreceivedbyaccount none
< 0
> getreceivedbyaccount t2 0
< 1.111e-05

After:

> getreceivedbyaddress $notMineAddress
< 0.00000000
> getreceivedbyaccount none
< 0.00000000
> getreceivedbyaccount t2 0
< 0.00001111

@jonasschnelli
Copy link
Contributor

Concept ACK.
What's the raw JSON differences? Is this a tiny API change?

@maflcko
Copy link
Member Author

maflcko commented Jul 8, 2016

What's the raw JSON differences? Is this a tiny API change?

yes, it is two different functions to generate the string. Before it used setFloat and oss << std::setprecision(16) << val;. After, it uses our wrapper function.

@jonasschnelli
Copy link
Contributor

I think we should do this (for 0.14) but would require a short part in the release notes.

@laanwj
Copy link
Member

laanwj commented Jul 8, 2016

utACK.

The numbers may be formatted slightly differently, but it's not an API change from the perspective of a client, it still returns a JSON number where a JSON number was returned. No need to mention this in release notes.

@maflcko
Copy link
Member Author

maflcko commented Jul 8, 2016

release notes

What would I write there?
Let's hope no implementation relies on some numbers being represented by scientific notation. Also note that getreceivedbyaddress (and all other calls) already use ValueFromAmount.

@jonasschnelli
Copy link
Contributor

[...] No need to mention this in release notes.

Okay for me.
utACK 477777f

@laanwj
Copy link
Member

laanwj commented Jul 8, 2016

Okay for me.

I think we should be careful to not add too much technical details to the explicit descriptions in the release notes. The release notes are already huge, every major release.

Also remember that every commit/PR already gets a mention with the title.

@laanwj laanwj merged commit 477777f into bitcoin:master Jul 11, 2016
laanwj added a commit that referenced this pull request Jul 11, 2016
477777f [rpcwallet] Don't use floating point (MarcoFalke)
@maflcko maflcko deleted the Mf1607-rpcFloat branch July 11, 2016 11:17
@maflcko maflcko added this to the 0.12.2 milestone Aug 26, 2016
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Nov 23, 2016
UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Mar 22, 2017
Github-Pull: bitcoin#8317
Rebased-From: 477777f
UdjinM6 added a commit to dashpay/dash that referenced this pull request Apr 11, 2017
* Implement BIP 9 GBT changes

- BIP9DeploymentInfo struct for static deployment info
- VersionBitsDeploymentInfo: Avoid C++11ism by commenting parameter names
- getblocktemplate: Make sure to set deployments in the version if it is LOCKED_IN
- In this commit, all rules are considered required for clients to support

* qa/rpc-tests: bip9-softforks: Add tests for getblocktemplate versionbits updates

* getblocktemplate: Explicitly handle the distinction between GBT-affecting softforks vs not

* getblocktemplate: Use version/force mutation to support pre-BIP9 clients

* Don't use floating point

Github-Pull: bitcoin#8317
Rebased-From: 477777f

* Send tip change notification from invalidateblock

This change is needed to prevent sync_blocks timeouts in the mempool_reorg
test after the sync_blocks update in the upcoming commit
"[qa] Change sync_blocks to pick smarter maxheight".

This change was initially suggested by Suhas Daftuar <[email protected]>
in bitcoin#8680 (comment)

Github-Pull: bitcoin#9196
Rebased-From: 67c6326

* torcontrol: Explicitly request RSA1024 private key

When generating a new service key, explicitly request a RSA1024 one.

The bitcoin P2P protocol has no support for the longer hidden service names
that will come with ed25519 keys, until it does, we depend on the old
hidden service type so make this explicit.

See bitcoin#9214.

Github-Pull: bitcoin#9234
Rebased-From: 7d3b627

* Bugfix: FRT: don't terminate when keypool is empty

Github-Pull: bitcoin#9295
Rebased-From: c24a4f5

* add fundrawtransaction test on a locked wallet with empty keypool

Github-Pull: bitcoin#9295
Rebased-From: 1a6eacb
thokon00 pushed a commit to faircoin/faircoin that referenced this pull request Apr 17, 2018
zkbot added a commit to zcash/zcash that referenced this pull request Apr 23, 2018
Bech32 encoding support and t-addr encoding refactor

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7922
- bitcoin/bitcoin#7825
- bitcoin/bitcoin#8317
- bitcoin/bitcoin#9804
  - Only the commit that changed `base58.cpp`
- bitcoin/bitcoin#11117
- bitcoin/bitcoin#11259
- bitcoin/bitcoin#11167
  - Only the first three commits (the fourth commit depends on #2390, later ones are SegWit-specific).

Part of #3058.
zkbot added a commit to zcash/zcash that referenced this pull request May 1, 2018
Upstream encoding cleanups

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7922
- bitcoin/bitcoin#7825
- bitcoin/bitcoin#8317
- bitcoin/bitcoin#9804
  - Only the commit that changed `base58.cpp`

Precursor to #3202.
zkbot added a commit to zcash/zcash that referenced this pull request May 1, 2018
Upstream encoding cleanups

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7922
- bitcoin/bitcoin#7825
- bitcoin/bitcoin#8317
- bitcoin/bitcoin#9804
  - Only the commit that changed `base58.cpp`

Precursor to #3202.
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants