feat(quorum): private transaction support #2293
Conversation
aldousalvarez
requested review from
petermetz,
takeutak,
izuru0,
jagpreetsinghsasan,
VRamakrishna and
sandeepnRES
as code owners
There was a problem hiding this comment.
LGTM, just write a detailed commit message for this and create follow-up issues to address the issue of privateUrl being truely optional. Right now we are passing privateUrl in all the instances of QuorumConnector even though we don't need to do private transactions everywhere.
The follow up issue can also include 2 test cases, one showcasing private tx (the newly created file in this PR) and a seperate test file showing public tx (a similar file as packages/cactus-plugin-ledger-connector-quorum/src/test/typescript/integration/plugin-ledger-connector-quorum/deploy-contract/v2.3.0-deploy-contract-from-json-private.test.ts, but just doing the public tx). Also that issue can include renaming of this privateUrl to something more specific, maybe a rpcApiHttpHost and another field being privacyEnabled set as true/false.
@petermetz are the expectations from the follow-up issues fine? (Or anything more required, given this PR being the current state of enabling private tx)
.../cactus-plugin-ledger-connector-quorum/src/main/typescript/plugin-ledger-connector-quorum.ts
Outdated
Show resolved
Hide resolved
aldousalvarez
force-pushed
the
quorum-private-transaction
branch
from
eba671e
to
7fea3a0
Compare
aldousalvarez
requested review from
jagpreetsinghsasan
and removed request for
petermetz,
VRamakrishna,
sandeepnRES and
izuru0
jagpreetsinghsasan
requested review from
petermetz,
VRamakrishna,
sandeepnRES and
izuru0
...gin-ledger-connector-quorum/deploy-contract/v2.3.0-deploy-contract-from-json-private.test.ts
Outdated
Show resolved
Hide resolved
@jagpreetsinghsasan Sorry for the slow response! We do need a good test case verifying that the private transactions that the connector can now allegedly run actually are private (otherwise the implementation in the connector is broken). |
aldousalvarez
force-pushed
the
quorum-private-transaction
branch
from
229711f
to
3ac6a46
Compare
|
@aldousalvarez I see some commits being pushed into the PR after the last review comment, if you think its ready, please |
|
@jagpreetsinghsasan I tried to update my branch with the latest commit at that time. my apologies I am still working on it as of now. Thank you |
aldousalvarez
force-pushed
the
quorum-private-transaction
branch
from
3ac6a46
to
21bfd35
Compare
----------------------------------------- - Added v2.3.0-deploy-contract-from-json-private.test.ts that would support private transaction test for Quorum. - Added a QuorumPrivateTransactionConfig on openapi.json - Added Web3JsQuorum on plugin-ledger-connector-quorum and added a transact private method to be able to proceed with the private transaction. - Currently the privateUrl in the test is being truely optional and we need to address this in the future. - We are just passing privateUrl in all the instances even though we don't need to do private transactions everywhere. Fixes hyperledger-cacti#951 Co-authored-by: Travis Payne <travis.payne@accenture.com> Co-authored-by: johnhomantaring <john.h.o.mantaring@accenture.com> Co-authored-by: jagpreetsinghsasan <jagpreet.singh.sasan@accenture.com> Co-authored-by: aldousalvarez aldousss.alvarez@gmail.com Co-authored-by: Peter Somogyvari <peter.somogyvari@accenture.com> Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
petermetz
force-pushed
the
quorum-private-transaction
branch
from
21bfd35
to
00bbaa4
Compare
There was a problem hiding this comment.
@aldousalvarez @jagpreetsinghsasan I pushed some minor changes to greatly reduce the diff and also did a rebase onto upstream/main. LGTM as-is now. Thank you all!
Fixes #951
Co-authored-by: Travis Payne travis.payne@accenture.com
Co-authored-by: johnhomantaring john.h.o.mantaring@accenture.com
Co-authored-by: jagpreetsinghsasan jagpreet.singh.sasan@accenture.com
Co-authored-by: aldousalvarez aldousss.alvarez@gmail.com