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

rm DB interface from privacyenabledstate pkg #1242

Merged
merged 2 commits into from May 14, 2020

Conversation

cendhu
Copy link
Contributor

@cendhu cendhu commented May 12, 2020

Type of change

  • Improvement (improvement to code)

Description

As part of FAB-10794, we would be making changes to LoadCommittedVersions(). Before doing that, we remove DB interface from the privacyenabledstate pkg (which includes LoadCommittedVersions() API) as it has a single implementation only. Only when we need to have multiple implementations, we need an interface. Otherwise, it just adds unnecessary difficulties during code navigation and casting such as db.(*privacyenabledstate.CommonStorageDB) in the test.

As we remove DB interface, the filename db.go does not make sense. Hence, we rename it to types.go as it holds all private data related types. Further, db_test.go contains tests that
should be in common_storage_db_test.go. Hence, we also move those test to the correct location and rename db_test.go to types_test.go.

Further, we have

  1. renamed CommonStorageDB to DB
  2. removed the DBProvider interface,
  3. removed the term CommonStorage from variables
  4. renamed common_storage_db.go to db.go and common_storage_db_test.go to db_test.go

Additional details

Two commits have been pushed to ease the review process.

Related issues

FAB-17880

@cendhu cendhu requested a review from a team as a code owner May 12, 2020 15:57
Copy link
Contributor

@manish-sethi manish-sethi left a comment

Choose a reason for hiding this comment

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

Discussed offline with @cendhu for some further changes - so, holding this for now.

As part of FAB-10794, we would be making changes to
LoadCommittedVersions(). Before doing that, we remove unnecessary
interface from the privacyenabledstate which includes
LoadCommittedVersions() API.

After we have removed privacyenabledstate.DB interface, the filename
db.go does not make sense. Hence, we rename it to types.go as it holds
all private data related types. Further, db_test.go contains tests that
should be in common_storage_db_test.go. Hence, we also move those test
to the correct location and rename db_test.go to types_test.go.

Further, we have renamed CommonStorageDB to DB, removed
DBProvider interface, and removed the term CommonStorage from variables.

FAB-17880

Signed-off-by: senthil <cendhu@gmail.com>
Signed-off-by: senthil <cendhu@gmail.com>
Copy link
Contributor

@manish-sethi manish-sethi left a comment

Choose a reason for hiding this comment

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

Just some nit comments on a couple variable names. But would not like to hold this PR for that.

@@ -319,7 +319,7 @@ func (p *Provider) open(ledgerID string) (ledger.PeerLedger, error) {
p.collElgNotifier.registerListener(ledgerID, pvtdataStore)

// Get the versioned database (state database) for a chain/ledger
vDB, err := p.vdbProvider.GetDBHandle(ledgerID)
db, err := p.dbProvider.GetDBHandle(ledgerID)
Copy link
Contributor

Choose a reason for hiding this comment

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

In the lines of above comment, statedb may be a better choice, if not vDB. Plain db looses the context.

vdbProvider privacyenabledstate.DBProvider
dbProvider *privacyenabledstate.DBProvider
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that we have more than one stores, I feel that the earlier name vdbProvider was clearer. If you want to change, then statedbProvider may be a better choice in order to retain the context.

@manish-sethi manish-sethi merged commit 6316b46 into hyperledger:master May 14, 2020
@cendhu
Copy link
Contributor Author

cendhu commented May 14, 2020

Thanks, @manish-sethi I agree with the suggested changes and have created https://jira.hyperledger.org/browse/FAB-17896 to address the review comments.

@cendhu cendhu deleted the rm-db-interface branch May 14, 2020 16:31
xhens added a commit to xhens/fabric that referenced this pull request May 22, 2020
* tests for commit hashes with rollback and reset (hyperledger#1221)

The COMMIT_HASH is being added to each block both
in v1.1 and >= v1.4 As this hash has to be the same
on all peers, it is necessary to start adding this
hash from the same block on all peers. Hence, it is
necessary to reset the existing channels to the
genesis block inorder to add COMMIT_HASH. Otherwise,
the peer would not add COMMIT_HASH to blocks.

There are two new sets of tests:
(1) ensure that the COMMIT_HASH is being added to
the block once a peer has been reset to the genesis
block. Otherwise, COMMIT_HASH would not be added.
(2) assuming that COMMIT_HASH is already present,
doing a rollback/reset and reprocessing the block
should not result in different COMMIT_HASH (as we
assume no changes in the validation results). This
test also helps to ensure that any changes to the
code that computes the COMMIT_HASH do not break
the backward compatibility.

FAB-17832 #done

Signed-off-by: manish <manish.sethi@gmail.com>
Signed-off-by: senthil <cendhu@gmail.com>

Co-authored-by: manish <manish.sethi@gmail.com>

* [FAB-17862] Allow upgrade-dbs to drop all dbs in multiple runs (hyperledger#1236)

Administrators may need to rerun upgrade-dbs in some cases, e.g.,
if they forgot to pass the CouchDB variable CORE_LEDGER_STATE_STATEDATABASE,
it will not drop the CouchDB databases as part of upgrade-dbs.
Therefore, upgrade-dbs should be able to drop all dbs when
running multiple times.

Signed-off-by: Wenjian Qiao <wenjianq@gmail.com>

* Improve test coverege for errors (hyperledger#1068)

Signed-off-by: Svetoslav Blyahov <svetlio.blyahoff@gmail.com>

* test for commit hash computation's backward compatibility (hyperledger#1240)

We might change the code that computes the block commit hash
during refactoring or to improve the performance from time to time.
When changing the hash computation code, we need to ensure that
we are not changing the way commit hash has been computed in previous
releases. Hence, we add a test to ensure that commit hash
computation is always compatible with the first code added in v1.1

FAB-17878

Signed-off-by: senthil <cendhu@gmail.com>

* Add snapshot file creator and reader functions

This commit adds reusable functions for creating and reading
the snapshot files. These functions are expected to be used
by various components that would generate or consume the snapshot
files, such as blockstorage and statedb

Signed-off-by: manish <manish.sethi@gmail.com>

* Use snapshot file creator in blockstorage

This commit leverages the snapshot file creator
to generate snapshot files and computing hashes for TXIDs.

FAB-17870

Signed-off-by: manish <manish.sethi@gmail.com>

* [FAB-17433] Add _lifecycle function for querying approved chaincode (hyperledger#1220)

Currently, Fabric peer does not provide a function to query the details of
approved chaincode definitions.

On the other hand, Fabric admins will need to confirm/use the information
after approval for their operations.

This patch adds a function for querying the details of the chaincode
definition approved by the organization itself to `_lifecycle`,
so that Fabric peer can provide a function to query them.

Signed-off-by: Tatsuya Sato <Tatsuya.Sato@hal.hitachi.com>

* [FAB-17846] Add clear statements indicating that external chaincode builder will likely require custom packaging of peer image

Signed-off-by: Tatsuya Sato <Tatsuya.Sato@hal.hitachi.com>

* Externally built and launched chaincodes cleanup on signal

When a peer is terminated via SIGTERM or SIGINT, it should
ensure any externally built and launched chaincodes have
a chance to cleanup.

FAB-17155

Signed-off-by: Will Lahti <wtlahti@us.ibm.com>

* Remove launchDir from session and cleanup test

The implementation of the e2e test wasn't properly verifying that run
directories were getting cleaned up. These changes address that.

Also, instead of delegating cleanup of launchDir to the session as a
special case, the session was modified to allow arbitrary callbacks to
be called at exit prior to returning from wait. This is more generic
than wiring through the launch directory and is simple enough.

Signed-off-by: Matthew Sykes <sykesmat@us.ibm.com>

* Correct HSM environment variables

Signed-off-by: Brett Logan <brett.t.logan@ibm.com>

* [FAB-17438] Add links to CA deployment Guide from Fabric Deployment Guide

Signed-off-by: pama-ibm <pama@ibm.com>

* Fix misleading doc statement

The Kafka to Raft migration doc incorrectly implies that mutual TLS is
required when using Raft.  More correctly, Raft nodes will authenticate
with eachother by presenting a client certificate, as is done in mutual
TLS.  However, other clients may continue to authenticate without mutual
TLS.

Signed-off-by: Jason Yellick <jyellick@us.ibm.com>

* FAB-17874 serialize message delivery in raft UTs

This commit introduces a channel to buffer ingress
messages for a chain, so they are consumed with
the same order.

Signed-off-by: Jay Guo <guojiannan1101@gmail.com>

* Remove references to libtool

The package we use for pkcs11 no longer uses libtool to interact with
shared libraries so we no longer need the prereq for development.

Signed-off-by: Matthew Sykes <sykesmat@us.ibm.com>

* Cleanup macOS prerequisites and instructions

Signed-off-by: Matthew Sykes <sykesmat@us.ibm.com>

* [FAB-17886] Fix default value of `sequence` parameter to empty (hyperledger#1262)

Currently, default value of `sequence` parameter for `peer chaincode lifecycle`
commands is set to `1`.

However, the default value should be modified to `0` that means empty for
the following several reasons.

- According to `Validate()` in `approveformyorg.go`, this function is
implemented assuming that `sequence` is `0`. This is inconsistent with
the default value `1`.

- The current default value `1` enables Fabric users `approveformyorg` without
specifying the sequence number. Not forcing the specification of a sequence
number may cause operation mistakes (e.g, updating the package-id for
sequence 1 by mistake) when running `approveformyorg` in the second and
subsequent times.

- The default value is preferably `0` to reuse the same parameter `sequence`
in CLI commands to query the details of approved chaincode definitions.

Signed-off-by: Tatsuya Sato <Tatsuya.Sato@hal.hitachi.com>

* rm DB interface from privacyenabledstate pkg (hyperledger#1242)

rm privacyenabledstate.DB interface

As part of FAB-10794, we would be making changes to
LoadCommittedVersions(). Before doing that, we remove unnecessary
interface from the privacyenabledstate which includes
LoadCommittedVersions() API.

After we have removed privacyenabledstate.DB interface, the filename
db.go does not make sense. Hence, we rename it to types.go as it holds
all private data related types. Further, db_test.go contains tests that
should be in common_storage_db_test.go. Hence, we also move those test
to the correct location and rename db_test.go to types_test.go.

Further, we have renamed CommonStorageDB to DB, removed
DBProvider interface, and removed the term CommonStorage from variables.

FAB-17880

Signed-off-by: senthil <cendhu@gmail.com>

rename common_storage_db.go to db.go

Signed-off-by: senthil <cendhu@gmail.com>

* Fix some typos

Signed-off-by: Matthew Sykes <sykesmat@us.ibm.com>

* Reset golang.org/x/sys to commit ef0ce1748380

A breaking change was introduced to the golang.org/x/sys/windows package
API that causes compilation failures in docker packages indirectly
consumed by Fabric. While the docker code has been updated to
accommodate the broken API, the change is not available in any release.

Instead of using the development branch of docker, this change points
the golang.org/x/sys dependency to the commit prior to the API change.

Signed-off-by: Matthew Sykes <sykesmat@us.ibm.com>

* Update vendored fabric-config version and associated integration test (hyperledger#1265)

* Update vendored fabric-config version

Pull in recent changes to the API.

FAB-17893

Signed-off-by: Will Lahti <wtlahti@us.ibm.com>

* Update configtx integration test to use new APIs

FAB-17893 #done

Signed-off-by: Will Lahti <wtlahti@us.ibm.com>

* Enable stateleveldb for scanning for snapshot files generation

This commit
 - Introduces a function in the statedb interface for getting access to
   an iterator that can be used for scanning the entire state,
   including private data hashes for a particular channel

- Provides the implementation of the above mentioned function for the
  leveldb based statedb.

FAB-17885

Signed-off-by: manish <manish.sethi@gmail.com>

* Replace link to removed topic with link to relavent image

Signed-off-by: NIKHIL E GUPTA <negupta@us.ibm.com>

* Update master references for v1.4.7

Update bootstrap.sh and docs for v1.4.7 release

Signed-off-by: David Enyeart <enyeart@us.ibm.com>

* minor refactoring of pvtstatepurgemgmt (hyperledger#1252)

* rm PurgeMgr and expiryKeeper interface

As we have only one implementation of both
PurgeMgr and expiryKeeper, we remove these
interfaces.

FAB-17884

Signed-off-by: senthil <cendhu@gmail.com>

* minor rearrangement of types

All types are moved to the top and kept in order.

Signed-off-by: senthil <cendhu@gmail.com>

* rm the term bookkeeping

To be explicit, we remove the term bookkeeping from
the pvtstatepurgemgmt and replace it with expiryKeeper
or expiryInfo as appropriate

Signed-off-by: senthil <cendhu@gmail.com>

* Merge first component of create channel tutorial

Signed-off-by: NIKHIL E GUPTA <negupta@us.ibm.com>

* Clarify error message when lscc chaincode install fails during build (hyperledger#1276)

When an LSCC chaincode install fails due to an error building the
chaincode, the chaincode will remain installed on the peer. This commit
updates the error message to indicate this behavior since it is (for better
or worse) working as designed.

FAB-17784

Signed-off-by: Will Lahti <wtlahti@us.ibm.com>

* [DocUpdate]configtxlator decode/common.ConfigUpdate

decode to json is using decode/common.ConfigUpdate routine

Signed-off-by: david <david-khala@hotmail.com>
Change-Id: I85d4578007a6dcb04a35561f657b8290b2cb7177

* Remove adding PWD to path in tutorials

Signed-off-by: NIKHIL E GUPTA <negupta@us.ibm.com>

* [FAB-17865] Update Fabric upgrade doc for dropping CouchDB

The "upgrade-dbs" peer command has been enhanced to automatically
drop the CouchDB state database as of v2.2.0. Update the upgrade
document accordingly.

Signed-off-by: Wenjian Qiao <wenjianq@gmail.com>

* cleanup code

Signed-off-by: hello2mao <hello2mao@gmail.com>

* Should use Debugf instead of Debug

Signed-off-by: bjzhang <bjzhang1991@gmail.com>

* Add export to deploy CC tutorial

Signed-off-by: NIKHIL E GUPTA <negupta@us.ibm.com>

* FAB-17839 Ch.Part.API: List channels REST handler (hyperledger#1232)

* FAB-17839 Ch.Part.API: List REST handler

Develop the following handler functionality.

List all channels:
`GET /participation/v1/channels`.

List a single handler:
`GET /participation/v1/channels/my-channel`.

Define the interface the Registrar will implement,
place skeleton methods in it.

Signed-off-by: Yoav Tock <tock@il.ibm.com>
Change-Id: I4ce65b406ccbbbe8e1dbd4a95680c3561f5fc7c9

* fix gofmt

Signed-off-by: Yoav Tock <tock@il.ibm.com>
Change-Id: I4836ff6dc3101c1ceaeb98db8234ee732f511717

* Review comments: split types to leaf package

Split the types used by packages multichannel & channelparticipation
to a leaf package, in order to minimize dependencies.

Signed-off-by: Yoav Tock <tock@il.ibm.com>
Change-Id: I0b9ea17fd85f1f5bf1aba3ea8be80fc7b19e190f

* Review comments: improve ChannelManagement interface

Streamline the ChannelManagement interface return types and type names.

Signed-off-by: Yoav Tock <tock@il.ibm.com>
Change-Id: I67e105f9e7e315c1ce6f5b16898de4aa94e609eb

* Review comments: use gorilla/mux to route requests

Instead of parsing and matching the path, use
gorilla/mux to route requests and identify the channel-id.

Signed-off-by: Yoav Tock <tock@il.ibm.com>
Change-Id: I6739fe027fab523a0a26a25838a36e42cc7f1469

* Review comments: use gorilla/mux to match headers

Signed-off-by: Yoav Tock <tock@il.ibm.com>
Change-Id: I179680a0556fef7110e69c0612687757844466e0

* Review comments: streamline types and return values

Change return values to non-pointer type.

Streamline ChannelList:
- Drop Size,
- Change Channels slice to carry app-channels only,
- Change SystemChannel to ChannelInfoShort.

Drop BlockHash from ChannelInfo.

Signed-off-by: Yoav Tock <tock@il.ibm.com>
Change-Id: I24c6ecccb58d95c307f03332ef0a4d2c06a063aa

* Review comments: rename methods

Signed-off-by: Yoav Tock <tock@il.ibm.com>
Change-Id: Ib5b88ab5bc63e4c3ed3ef1cd44c6a17e755b8b91

* Address issues flagged by shellcheck

A few new shellcheck issues have crept into the scripts. Make the
changes recommended by static analysis.

Signed-off-by: Matthew Sykes <sykesmat@us.ibm.com>

* Add integration-test-prereqs target and update doc

The `docker-test-prereqs` target used in the doc was never committed.
Replace it with `integration-test-prereqs`.

Signed-off-by: Matthew Sykes <sykesmat@us.ibm.com>

* [DocUpdate] chaincode package tutorial

- speak out the source tar name required in package

Signed-off-by: david <david-khala@hotmail.com>
Change-Id: I9843a22ffd474558d30bf7d085665d5deeeb1620

* [FAB-17793] Generate db name to namespace mapping for state couchdb (hyperledger#1268)

This PR generates namespace to namespaceDBInfo mapping for state couchdb and
stores the mapping data in channel’s metadata db. Due to couchdb's
length restriction on db names, the mapping is needed to drop all
the databases for a channel as well as snapshot support.

Signed-off-by: Wenjian Qiao <wenjianq@gmail.com>

* [FAB-17844] Copy symlinks as-is in external builder output

Currently, the external builder code does not check
for symlinks in build output when copying them. This
results in the resolved files being copied as files
instead of symlinks.

Node.js chaincode relies on certain symlinks that npm
creates in the node_modules tree. There may be other
chaincode scenarios that rely on symlinks.

This commit changes the external builder code so that
it tests for symlinks, and copies them as symlinks
instead of copying them as files into the destination
directory.

Signed-off-by: Simon Stone <sstone1@uk.ibm.com>

* Fix the installed binary list in the document

Signed-off-by: 5pecia1 <pdpxpd@gmail.com>

* FAB-17889 Ch.Part.API: Registrar channel list (hyperledger#1290)

* FAB-17889 Ch.Part.API: Registrar channel list

Allow the registrar to produce a list of all channels.

Note that all access to the system channel name is now under lock,
because with the introduction of this API it is no longer immutable.

Signed-off-by: Yoav Tock <tock@il.ibm.com>
Change-Id: I4993372eb9961af528cd35d868e53dec6d53ae74

* Review comments: simplify code

Signed-off-by: Yoav Tock <tock@il.ibm.com>
Change-Id: I2499fcd9b21c2689829d275f4c28a2104d0d460a

* Review comments: simplify more

Signed-off-by: Yoav Tock <tock@il.ibm.com>
Change-Id: Idcd63309c513e3d0848a3bd9e3b885687781b809

* fix test network docs typo

Signed-off-by: zhuzeyu <zhuzeyu0409@gmail.com>

* Change default CouchDB maxRetriesOnStartup to 10

Change default maxRetriesOnStartup from 12 to 10.
Also fix retries so that there is no sleep after the last attempt,
and improve the retry message to make it clear how many attempts and when retries are exhausted.
The changes result in peer start failure after 2 minutes rather than 16 minutes.

Signed-off-by: David Enyeart <enyeart@us.ibm.com>

* Fix script help text in the test network document

Signed-off-by: 5pecia1 <pdpxpd@gmail.com>

* Fix Second-Chance Algorithm issue.

New added item's referenced bit should be set to false
when cache is not full.

Signed-off-by: Hongbin Mao <hello2mao@gmail.com>

* Errors should be checked when orderer grpc server is serving requests

Signed-off-by: wuyingjun <wuyingjun@cmss.chinamobile.com>

* Add ExportConfigHistory() API to confighistoryDB (hyperledger#1288)

This commit adds ExportConfigHistory() API to the
confighistoryDB retriever. It utilizes snapshot file
writer to dump keys & values stored in the
confighistoryDB to a snapshot file.

Signed-off-by: senthil <cendhu@gmail.com>

* [FAB-17434] Add `queryapproved` command to query the details of the approved chaincode definition (hyperledger#1243)

Currently, Fabric peer does not provide a function to query the details of
approved chaincode definitions.

On the other hand, Fabric admins will need to confirm/use the information
after approval for their operations.

This patch adds `queryapproved` command for querying the details of
the chaincode definition approved by the organization itself to peer CLI,
so that Fabric peer can provide a function to query them.

Signed-off-by: Tatsuya Sato <Tatsuya.Sato@hal.hitachi.com>

* Improve error message when CLI cannot send to orderer (hyperledger#1308)

from:
"could not send"
to:
"could not send to orderer node"

This change will assist users with troubleshooting error messages from peer CLI.

Signed-off-by: David Enyeart <enyeart@us.ibm.com>

* Generate statedb snapshot files

This commit introduces a function in the statedb that exports the
public state and private state hashes from statedb into a separate
set of snapshot files in a deterministic format.

FAB-17902

Signed-off-by: manish <manish.sethi@gmail.com>

* [FAB-17435] Add integration tests for `queryapproved` command (hyperledger#1309)

Currently, Fabric peer does not provide a function to query the details of
approved chaincode definitions.

On the other hand, Fabric admins will need to confirm/use the information
after approval for their operations.

Other patches add `queryapproved` command for querying the details of
the approved chaincode definition to Fabric peer CLI
(Refer to FAB-17433 and FAB-17434).

This patch adds integration tests for the command.

Signed-off-by: Tatsuya Sato <Tatsuya.Sato@hal.hitachi.com>

* [FAB-17436] Add documents for `queryapproved` command (hyperledger#1310)

Currently, Fabric peer does not provide a function to query the details of
approved chaincode definitions.

On the other hand, Fabric admins will need to confirm/use the information
after approval for their operations.

Other patches add `queryapproved` command for querying the details of
the approved chaincode definition to Fabric peer CLI
(Refer to FAB-17433 and FAB-17434).

This patch adds some documents on how to use the CLI command.

Signed-off-by: Tatsuya Sato <Tatsuya.Sato@hal.hitachi.com>

* Provide more context in identity deserialization warning msg (hyperledger#1256)

FAB-14099 #done

Signed-off-by: Will Lahti <wtlahti@us.ibm.com>

* Update Prereqs for Fabric users

Signed-off-by: pama-ibm <pama@ibm.com>

* Contain the dependency on bccsp

This commit
- Removes the dependency on bccsp package from much deeper code and
contains the dependency only at the level of top level - kvledger

- Though the refactoring is straight forward, still this commit adds
a test to make sure that the code that generates and validates the
merkle tree for the range queries is backward compatible. This test
was long due that can help catch any incompatible changes in the code or
the hash function used

Signed-off-by: manish <manish.sethi@gmail.com>

Co-authored-by: Senthil Nathan N <cendhu@users.noreply.github.com>
Co-authored-by: manish <manish.sethi@gmail.com>
Co-authored-by: wen <wenjianq@gmail.com>
Co-authored-by: Svetoslav <betasve@users.noreply.github.com>
Co-authored-by: Tatsuya Sato <Tatsuya.Sato@hal.hitachi.com>
Co-authored-by: Will Lahti <wtlahti@us.ibm.com>
Co-authored-by: Matthew Sykes <sykesmat@us.ibm.com>
Co-authored-by: Brett Logan <brett.t.logan@ibm.com>
Co-authored-by: pama-ibm <pama@ibm.com>
Co-authored-by: Jason Yellick <jyellick@us.ibm.com>
Co-authored-by: Jay Guo <guojiannan1101@gmail.com>
Co-authored-by: NIKHIL E GUPTA <negupta@us.ibm.com>
Co-authored-by: David Enyeart <enyeart@us.ibm.com>
Co-authored-by: david <david-khala@hotmail.com>
Co-authored-by: hello2mao <hello2mao@gmail.com>
Co-authored-by: bjzhang <bjzhang1991@gmail.com>
Co-authored-by: Yoav Tock <tock@il.ibm.com>
Co-authored-by: Simon Stone <sstone1@uk.ibm.com>
Co-authored-by: 5pecia1 <pdpxpd@gmail.com>
Co-authored-by: zhuzeyu <zhuzeyu0409@gmail.com>
Co-authored-by: wuyingjun <wuyingjun@cmss.chinamobile.com>
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

2 participants