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.

Sign up for GitHub

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

Jump to bottom

Sequencing relay requests #902

Merged
merged 15 commits into from
Jul 12, 2019
Merged

Sequencing relay requests #902

merged 15 commits into from
Jul 12, 2019

Conversation

dimpar
Copy link
Contributor

@dimpar dimpar commented Jul 4, 2019 •
edited
Loading

Closes: #884

In this PR we need to block upcoming requests from processing if there is one in progress. After a node generates a relay entry, then it can again start processing a new request. These are the rules when we block a new relay request from processing:

relaySigningInProgess  | block.number > relayEntryTimeout | blocking a new relay request?
true                   |           true                   | no
true                   |           false                  | yes
false                  |           true                   | no
false                  |           false                  | no

@dimpar
Adding a relay request timeout to block processing upcoming requests
14ef067 
- Set relayRequestTimeout to 17
- Modified sign(..) function by adding require(..) to handle timeout
- Added tests
@dimpar
Copy link
Contributor Author

dimpar commented Jul 4, 2019

Testing for `relayRequestTimeout` blocks calculation: 47579 - 47562 = 17

New relay entry requested [&{RequestID:+2 Payment:+0 PreviousEntry:+10920102476789591414949377782104707130412218726336356788412941355500907533021 Seed:+53067088278316309045121968048462724729152109215234493179134844471987707841858 GroupPublicKey:[148 201 102 80 168 175 134 22 57 88 174 102 140 6 86 157 161 253 198 211 221 66 2 185 54 173 121 213 147 68 47 34 3 106 162 84 138 240 215 67 22 146 199 110 198 201 30 30 165 163 237 77 177 130 240 136 9 94 34 95 134 202 253 107] BlockNumber:47562}]
[member:2] Waiting for block 47562 to start execution...
[member:2, state:*entry.signatureShareState] Transitioning to a new state at block [47562]...
[member:4] Waiting for block 47562 to start execution...
[...]

[member:2, state:*entry.signatureCompleteState] Transitioned to new state
[member:2, state:*entry.entrySubmissionState] Transitioning to a new state at block [47566]...
error creating threshold signature for request [2]: [failed to initiate new state [blocking member index [2]]]
[member:1, state:*entry.signatureCompleteState] Transitioned to new state
[member:3, state:*entry.signatureCompleteState] Transitioned to new state
[member:3, state:*entry.entrySubmissionState] Transitioning to a new state at block [47566]...
[member:1, state:*entry.entrySubmissionState] Transitioning to a new state at block [47566]...
error creating threshold signature for request [2]: [failed to initiate new state [blocking member index [3]]]
[member:5, state:*entry.signatureCompleteState] Transitioned to new state
[member:5, state:*entry.entrySubmissionState] Transitioning to a new state at block [47566]...
error creating threshold signature for request [2]: [failed to initiate new state [blocking member index [1]]]
[member:4, state:*entry.signatureCompleteState] Transitioned to new state
[member:4, state:*entry.entrySubmissionState] Transitioning to a new state at block [47566]...
error creating threshold signature for request [2]: [failed to initiate new state [blocking member index [4]]]
[member:5] Waiting for block [47578] to submit...
[member:5] Submitting relay entry..
Group selection started [&{NewEntry:+59349521601226944235574872895907257508492248931772837984180811443058308273889 RequestID:+2 Seed:+53067088278316309045121968048462724729152109215234493179134844471987707841858 BlockNumber:47579}]
New relay entry generated [&{RequestID:+2 Value:+59349521601226944235574872895907257508492248931772837984180811443058308273889 GroupPubKey:[148 201 102 80 168 175 134 22 57 88 174 102 140 6 86 157 161 253 198 211 221 66 2 185 54 173 121 213 147 68 47 34 3 106 162 84 138 240 215 67 22 146 199 110 198 201 30 30 165 163 237 77 177 130 240 136 9 94 34 95 134 202 253 107] PreviousEntry:+10920102476789591414949377782104707130412218726336356788412941355500907533021 Timestamp:2019-07-02 12:55:25.804585 +0000 UTC Seed:+53067088278316309045121968048462724729152109215234493179134844471987707841858 BlockNumber:47579}]
New relay entry generated at block number [47579]
[member:5] Relay entry for request [2] successfully submitted at block [47579]
[...]

@pdyraga pdyraga requested review from ngrinkevich and pdyraga July 4, 2019 15:33
@pdyraga
Copy link
Member

pdyraga commented Jul 4, 2019

Does it Refs or Closes #884?

@dimpar
Copy link
Contributor Author

dimpar commented Jul 8, 2019

Does it Refs or Closes #884?

changed to closes

@dimpar dimpar mentioned this pull request Jul 10, 2019
Closed
@pdyraga
Copy link
Member

pdyraga commented Jul 11, 2019

@dimpar I think it makes sense to rename this PR to e.g. "Serialize relay requests". Timeout is just an implication of the fact we are serializing them.

@pdyraga pdyraga mentioned this pull request Jul 11, 2019
Merged
dimpar added 2 commits July 11, 2019 15:53
@dimpar
Changing names around relay timeout and breaking down timeout value
43871e5 
- Modified the timeout variables to reflect signing and publication.
- Broke down the value of timeout. Signing time + publication deadline.
- Changed before to beforeEach in the test
@dimpar
Merge remote-tracking branch 'origin' into adding-relay-request-timeout
b58eb01
@dimpar
Copy link
Contributor Author

dimpar commented Jul 11, 2019 •
edited
Loading

Right, I agree that "Serialize relay request" is a more generic description, but tbh at first I was a bit confused because I thought of some objects converting to a byte stream, just like in Java. How about "Sequencing relay requests"?

@dimpar
Fixing solidity tests
83e5355 
- Changed the message from 'required'.
- relayRequestTimeout -> relayEntryTimeout
@dimpar dimpar changed the title Adding a relay request timeout to block processing upcoming requests Sequencing relay requests Jul 12, 2019
dimpar added 2 commits July 12, 2019 14:20
@dimpar
Renaming ..RelayRequestTimeout to RelayEntryTimeout.
e3b2d97 
- Renamed the contract name.
- Renamed the file.
@dimpar
Adding more gas to fix a 'revert' error and chaning 'value'
824594c 
- Added more gas, because of a 'revert' failure when calling a `sign`
function.
- Changed the value for the transaction in wei.
@dimpar
Copy link
Contributor Author

dimpar commented Jul 12, 2019

Tested:

  • genesis looked good
  • relay request looked good too
truffle exec scripts/query-beacon.js --network local

Number of active groups: 2
Last relay entry: 60363446453050641565982799916615533980909969969341377909501373075142852260747

@pdyraga
Copy link
Member

pdyraga commented Jul 12, 2019

Tested the following scenario:

  • deployed contracts
  • ran genesis, waited for the first group to be formed
  • requested entry, observed it's processed properly
  • tried to request another entry multiple times before the first one finished - my TX was rejected, as expected
  • once the requested entry has been processed I requested another one at block 89931
  • killed the client before entry has been published
  • restarted client
  • tried to request for a new entry several times - I couldn't do it until block 89955
  • new entry request at block 89955 has been accepted and properly processed

Works as expected!

@pdyraga pdyraga merged commit 54a46e5 into master Jul 12, 2019
@pdyraga pdyraga deleted the adding-relay-request-timeout branch July 12, 2019 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ngrinkevich ngrinkevich ngrinkevich left review comments

@pdyraga pdyraga pdyraga approved these changes

Assignees
No one assigned
Labels
⛓️ solidity
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Serialize relay requests
3 participants
@dimpar @pdyraga @ngrinkevich