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

Merge bitpay's upstream #1231

Closed
wants to merge 20 commits into from
Closed

Conversation

snogcel
Copy link

@snogcel snogcel commented Dec 29, 2016

Braydon Fuller and others added 20 commits July 14, 2016 13:56
There was a previous assumption that blockindex would be quite small. With addressindex
and spentindex enabled the blockindex is much larger and the amount of cache allocated for
it should also increase. Furthermore, enabling compression should decrease the amount of
disk space required and less data to write/read. The default leveldb max_open_files is set to
1000, for the blockindex the default is set to 1000 with compression. The 64 value that is
current is kept for the utxo database and does not enable compression. Two additional options
are added here to be able to configure the values for leveldb and the block index:

- `-dbmaxopenfiles` A number of files for leveldb to keep open
- `-dbcompression` Boolean 0 or 1 to enable snappy leveldb compression
since there is i/o that can happen when retrieving extended input
and output information, limiting the duration of the lock in getrawtransaction
can help improve concurrency
usage: ./wallet-utility -datadir=<directory>
help: ./wallet-utility -h
wallet-utility: extract addresses and private keys
Add options to configure block index database
logical timestamp indexing of block hashes
test: fix determinism of address index test
rpc: option to include chain info in address index results
rpc: add method to get address deltas from a block
@UdjinM6 UdjinM6 changed the title 0.12.1 bitcore-4 Merge bitpay's upstream Dec 29, 2016
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@schinzelh
Copy link

Are these features mission critical for 12.1 rollout?
12.1 branch is in feature freeze & bugfix mode, so I'd like to shift this to 12.2

@tgflynn
Copy link

tgflynn commented Dec 29, 2016

nACK

This should not go into 12.1 unless there are critical fixes, IMO.

@snogcel
Copy link
Author

snogcel commented Dec 29, 2016

My apologies I should have included the reasoning in the description. This is a fairly important one but more on the Evolution Backend side of things. Specifically it relates to Insight API / UI as well as bitcore-node-dash: dashpay/dashcore-node#5 (and more specifically to bitpay#29).

In short: the current release of bitcore-node-dash 3.1.3 is making reference to features that don't exist in dash core v0.12.1.x. We've merged up to https://github.com/bitpay/bitcoin/releases/tag/v0.12.1-bitcore.

Recently I noticed that insight-ui isn't pulling in "Latest Blocks" when using the latest build of bitcore-node-dash (you can see this at https://dev-test.dash.org). After some debugging I realized that is because it's making reference to an RPC command that isn't currently implemented in v0.12.1.x.

To fix that specific bug, I think only this commit would be needed: de6f29e

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After discussion in slack I guess, we are going to postpone this merge for later, so I'll just leave a note here to make sure we didn't forget about it, see inline comments.

@@ -119,6 +119,8 @@ static const bool DEFAULT_TXINDEX = true;
static const bool DEFAULT_ADDRESSINDEX = false;
static const bool DEFAULT_TIMESTAMPINDEX = false;
static const bool DEFAULT_SPENTINDEX = false;
static const unsigned int DEFAULT_DB_MAX_OPEN_FILES = 1000;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bitcoin Core had some problems with high number of open files in the past and they still have it at 64 so this change can be dangerous actually.

Let's make sure it works on every platform in all modes (sending a bunch of txes, reindexing, syncing from network) first or change this back to current values i.e.

static const unsigned int DEFAULT_DB_MAX_OPEN_FILES = 64;
static const bool DEFAULT_DB_COMPRESSION = false;

@schinzelh schinzelh added this to the 12.2 milestone Jan 13, 2017
@SCDeveloper
Copy link

Why is this being held back until version v0.12.2.x ?

This is not just an enchancement but a bug fix also.

@schinzelh
Copy link

This is not a bugfix, but very specific Bitpay features - we don't want to break our stable branch 2 weeks before launch.

@schinzelh schinzelh changed the base branch from v0.12.1.x to v0.12.2.x February 20, 2017 20:39
@UdjinM6 UdjinM6 removed this from the 12.2 milestone Mar 16, 2018
@nmarley
Copy link

nmarley commented Nov 10, 2018

I believe the features that were requested have now been merged in via other PRs -- @UdjinM6 ok to close this 2-year old PR?

@UdjinM6
Copy link

UdjinM6 commented Nov 10, 2018

@nmarley I don't think any of these changes were merged in dash core (c++) but since no one complained I guess it was fixed in dashcore-node (nodejs) somehow, right?

@nmarley
Copy link

nmarley commented Nov 10, 2018

Well I thought Jon wanted the indexes (e.g. address, timestamp indices) but I guess those probably already were merged previously. Looking closer, it seems like he needed an extra JSON argument to getblockhashes, but I'm also guessing this isn't needed any more (and Evo backend has changed since 2016, when it was still quite early).

@nmarley
Copy link

nmarley commented Nov 10, 2018

This particular PR can probably be closed regardless, and I think we should be able to open new ones for any individual things which might be needed. Scope of this is too large IMO.

@UdjinM6
Copy link

UdjinM6 commented Nov 10, 2018

Ok, closing

@UdjinM6 UdjinM6 closed this Nov 10, 2018
xdustinface pushed a commit to xdustinface/dash that referenced this pull request Dec 18, 2020
…llet

Add label API to wallet RPC.

This is one step towards dashpay#3816 ("Remove bolt-on account system") although it doesn't
actually remove anything yet.

These initially mirror the account functions, with the following differences:

- These functions aren't DEPRECATED in the help
- Help mentions 'label' instead of accounts. In the language used, labels are
  associated with addresses, instead of addresses associated with labels. (unlike
  with accounts.)
- Labels have no balance
  - No balances in `listlabels`
  - `listlabels` has no minconf or watchonly argument
- Like in the GUI, labels can be set on any address, not just receiving addreses
- Unlike accounts, labels can be deleted.
  Being unable to delete them is a common annoyance (see dashpay#1231).
  Currently only by reassigning all addresses using `setlabel`, but an explicit
  call `deletelabel` which assigns all address to the default label may make
  sense.

Thanks to Pierre Rochard for test fixes.
xdustinface pushed a commit to xdustinface/dash that referenced this pull request Dec 18, 2020
Add label API to wallet RPC.

This is one step towards dashpay#3816 ("Remove bolt-on account system") although it doesn't
actually remove anything yet.

These initially mirror the account functions, with the following differences:

- These functions aren't DEPRECATED in the help
- Help mentions 'label' instead of accounts. In the language used, labels are
  associated with addresses, instead of addresses associated with labels. (unlike
  with accounts.)
- Labels have no balance
  - No balances in `listlabels`
  - `listlabels` has no minconf or watchonly argument
- Like in the GUI, labels can be set on any address, not just receiving addreses
- Unlike accounts, labels can be deleted.
  Being unable to delete them is a common annoyance (see dashpay#1231).
  Currently only by reassigning all addresses using `setlabel`, but an explicit
  call `deletelabel` which assigns all address to the default label may make
  sense.

Thanks to Pierre Rochard for test fixes.
xdustinface pushed a commit to xdustinface/dash that referenced this pull request Dec 22, 2020
Add label API to wallet RPC.

This is one step towards dashpay#3816 ("Remove bolt-on account system") although it doesn't
actually remove anything yet.

These initially mirror the account functions, with the following differences:

- These functions aren't DEPRECATED in the help
- Help mentions 'label' instead of accounts. In the language used, labels are
  associated with addresses, instead of addresses associated with labels. (unlike
  with accounts.)
- Labels have no balance
  - No balances in `listlabels`
  - `listlabels` has no minconf or watchonly argument
- Like in the GUI, labels can be set on any address, not just receiving addreses
- Unlike accounts, labels can be deleted.
  Being unable to delete them is a common annoyance (see dashpay#1231).
  Currently only by reassigning all addresses using `setlabel`, but an explicit
  call `deletelabel` which assigns all address to the default label may make
  sense.

Thanks to Pierre Rochard for test fixes.
xdustinface pushed a commit to xdustinface/dash that referenced this pull request Dec 22, 2020
Add label API to wallet RPC.

This is one step towards dashpay#3816 ("Remove bolt-on account system") although it doesn't
actually remove anything yet.

These initially mirror the account functions, with the following differences:

- These functions aren't DEPRECATED in the help
- Help mentions 'label' instead of accounts. In the language used, labels are
  associated with addresses, instead of addresses associated with labels. (unlike
  with accounts.)
- Labels have no balance
  - No balances in `listlabels`
  - `listlabels` has no minconf or watchonly argument
- Like in the GUI, labels can be set on any address, not just receiving addreses
- Unlike accounts, labels can be deleted.
  Being unable to delete them is a common annoyance (see dashpay#1231).
  Currently only by reassigning all addresses using `setlabel`, but an explicit
  call `deletelabel` which assigns all address to the default label may make
  sense.

Thanks to Pierre Rochard for test fixes.
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Mar 12, 2022
Add label API to wallet RPC.

This is one step towards dashpay#3816 ("Remove bolt-on account system") although it doesn't
actually remove anything yet.

These initially mirror the account functions, with the following differences:

- These functions aren't DEPRECATED in the help
- Help mentions 'label' instead of accounts. In the language used, labels are
  associated with addresses, instead of addresses associated with labels. (unlike
  with accounts.)
- Labels have no balance
  - No balances in `listlabels`
  - `listlabels` has no minconf or watchonly argument
- Like in the GUI, labels can be set on any address, not just receiving addreses
- Unlike accounts, labels can be deleted.
  Being unable to delete them is a common annoyance (see dashpay#1231).
  Currently only by reassigning all addresses using `setlabel`, but an explicit
  call `deletelabel` which assigns all address to the default label may make
  sense.

Thanks to Pierre Rochard for test fixes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants