Skip to content
This repository has been archived by the owner on Jul 27, 2022. It is now read-only.

Problem: no runner for EDP-based tx-validation (fixes #1950) #1968

Merged
merged 1 commit into from
Jul 21, 2020
Merged

Problem: no runner for EDP-based tx-validation (fixes #1950) #1968

merged 1 commit into from
Jul 21, 2020

Conversation

tomtau
Copy link
Contributor

@tomtau tomtau commented Jul 20, 2020

Solution:

  • added a basic req-rep runner writing/reading from two vectors
    -- it's using std::sync::* primitives,
    because EnclaveProxy is synchronous +
    for some, it doesn't matter or may even be preferred
    (e.g. tokio Mutex isn't preferred if locks are short-lived
    and don't cross multiple await points)
  • temporary zmq server to faciliate tx-query
    -- because of the treading, EnclaveProxy also needed to
    accept lifetime of 'static
  • one extra variant in the enclave protocol (mainly used for testing)
  • old code temporarily hidden under "legacy" flag
  • since EDP runner only compiles on nightly,
    there's a fix/hack on Travis to exclude EDP

@tomtau tomtau requested a review from calvinaco as a code owner July 20, 2020 08:49
@tomtau
Copy link
Contributor Author

tomtau commented Jul 20, 2020

bors try

bors bot added a commit that referenced this pull request Jul 20, 2020
@bors
Try #1968:
f51869f
@bors
Copy link
Contributor

bors bot commented Jul 20, 2020

try

Build failed:

devashishdxt
devashishdxt reviewed Jul 21, 2020
View reviewed changes
Copy link
Collaborator

@devashishdxt devashishdxt left a comment

Choose a reason for hiding this comment

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

I'm not sure if using std::sync::* primitives in multithreaded async ecosystem is safe (this may lead to deadlocks). But, it may be safe if the lock does not cross an await point (if it does, then that may be an issue).

@tomtau
Copy link
Contributor Author

tomtau commented Jul 21, 2020

bors try

bors bot added a commit that referenced this pull request Jul 21, 2020
@bors
Try #1968:
60948bd
@bors
Copy link
Contributor

bors bot commented Jul 21, 2020

try

Build failed:

@tomtau
Copy link
Contributor Author

tomtau commented Jul 21, 2020

bors try

bors bot added a commit that referenced this pull request Jul 21, 2020
@bors
Try #1968:
1166dd6
@bors
Copy link
Contributor

bors bot commented Jul 21, 2020

try

Build failed:

@tomtau
Copy link
Contributor Author

tomtau commented Jul 21, 2020

bors retry

bors bot added a commit that referenced this pull request Jul 21, 2020
@bors
Try #1968:
3813f44
@bors
Copy link
Contributor

bors bot commented Jul 21, 2020

try

Build failed:

@tomtau
Copy link
Contributor Author

tomtau commented Jul 21, 2020

bors try

bors bot added a commit that referenced this pull request Jul 21, 2020
@bors
Try #1968:
0c0ee0b
@bors
Copy link
Contributor

bors bot commented Jul 21, 2020

try

Build failed:

@tomtau
Copy link
Contributor Author

tomtau commented Jul 21, 2020

bors try

bors bot added a commit that referenced this pull request Jul 21, 2020
@bors
Try #1968:
8fc1d8c
@codecov
Copy link

codecov bot commented Jul 21, 2020 •
edited
Loading

Codecov Report

Merging #1968 into master will decrease coverage by 0.52%.
The diff coverage is 0.98%.

@@            Coverage Diff             @@
##           master    #1968      +/-   ##
==========================================
- Coverage   65.79%   65.26%   -0.53%     
==========================================
  Files         203      206       +3     
  Lines       25638    25859     +221     
==========================================
+ Hits        16868    16877       +9     
- Misses       8770     8982     +212
Impacted Files Coverage Δ
chain-abci/src/app/app_init.rs 80.38% <0.00%> (-1.58%) ⬇️
chain-abci/src/app/commit.rs 95.16% <ø> (ø)
chain-abci/src/app/end_block.rs 100.00% <ø> (ø)
chain-abci/src/app/mod.rs 90.78% <ø> (ø)
chain-abci/src/app/query.rs 63.57% <ø> (ø)
chain-abci/src/app/rewards.rs 98.29% <ø> (ø)
chain-abci/src/app/validate_tx.rs 84.70% <ø> (ø)
chain-abci/src/enclave_bridge/edp/mod.rs 0.00% <0.00%> (ø)
chain-abci/src/enclave_bridge/edp/server.rs 0.00% <0.00%> (ø)
chain-abci/src/main.rs 0.00% <0.00%> (ø)
... and 10 more

@bors
Copy link
Contributor

bors bot commented Jul 21, 2020

try

Build failed:

@tomtau
Copy link
Contributor Author

tomtau commented Jul 21, 2020

bors retry

bors bot added a commit that referenced this pull request Jul 21, 2020
@bors
Try #1968:
00a3319
@bors
Copy link
Contributor

bors bot commented Jul 21, 2020

try

Build failed:

@tomtau
Copy link
Contributor Author

tomtau commented Jul 21, 2020

bors try

bors bot added a commit that referenced this pull request Jul 21, 2020
@bors
Try #1968:
37cac38
@tomtau tomtau force-pushed the switch/tx-validation-runner branch from 68ef1a4 to 77c2644 Compare July 21, 2020 08:11
@tomtau tomtau requested a review from devashishdxt July 21, 2020 08:12
@tomtau
Copy link
Contributor Author

tomtau commented Jul 21, 2020

I'm not sure if using std::sync::* primitives in multithreaded async ecosystem is safe (this may lead to deadlocks). But, it may be safe if the lock does not cross an await point (if it does, then that may be an issue).

@devashishdxt I simplified the locking there -- there's only one thread pushing the request at a time and waiting for a reply via a mpsc channel (send isn't blocking)

@bors
Copy link
Contributor

bors bot commented Jul 21, 2020

try

Build succeeded:

devashishdxt
devashishdxt approved these changes Jul 21, 2020
View reviewed changes
Copy link
Collaborator

@devashishdxt devashishdxt left a comment

Choose a reason for hiding this comment

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

LGTM

chain-abci/src/enclave_bridge/edp/mod.rs Outdated Show resolved Hide resolved
chain-abci/src/enclave_bridge/edp/mod.rs Outdated Show resolved Hide resolved
@tomtau tomtau force-pushed the switch/tx-validation-runner branch from 77c2644 to a3bcfb7 Compare July 21, 2020 10:42
@tomtau
Copy link
Contributor Author

tomtau commented Jul 21, 2020

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 21, 2020

Merge conflict.

@tomtau
Problem: no runner for EDP-based tx-validation (fixes crypto-com#1950)
7fc6ada 
Solution:
- added a basic req-rep runner writing one cursor-vector + sending back
reply in a channel
-- it's using std::sync::* primitives,
because EnclaveProxy is synchronous +
for some, it doesn't matter or may even be preferred
(e.g. tokio Mutex isn't preferred if locks are short-lived
and don't cross multiple await points)
- temporary zmq server to faciliate tx-query
-- because of the threading, EnclaveProxy also needed to
accept lifetime of 'static
- one extra variant in the enclave protocol (mainly used for testing)
- old code temporarily hidden under "legacy" flag
- since EDP runner only compiles on nightly,
there's a fix/hack on Travis to exclude EDP
@tomtau tomtau force-pushed the switch/tx-validation-runner branch from a3bcfb7 to 7fc6ada Compare July 21, 2020 13:22
@tomtau
Copy link
Contributor Author

tomtau commented Jul 21, 2020

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 21, 2020

Build succeeded:

@bors bors bot merged commit 48ddd62 into crypto-com:master Jul 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@devashishdxt devashishdxt devashishdxt approved these changes

@yihuang yihuang Awaiting requested review from yihuang

@leejw51crypto leejw51crypto Awaiting requested review from leejw51crypto

@linfeng-crypto linfeng-crypto Awaiting requested review from linfeng-crypto

@calvinaco calvinaco Awaiting requested review from calvinaco calvinaco is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tomtau @devashishdxt