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

Relay request timeout #897

Closed
wants to merge 82 commits into from
Closed

Relay request timeout #897

wants to merge 82 commits into from

Conversation

dimpar
Copy link
Contributor

@dimpar dimpar commented Jul 2, 2019

Refs: #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

First step as part of the refactoring this contract as immutable back-end
for the random beacon. Removing getters for the constants since these are
now auto-generated from public variables.
Backend contract should not track beacon entries and only needs to hold
the last entry used as a selection seed for the next group selection run.
This is useful in case of using specific stub contracts in tests.
Use 'before' instead of 'beforeEach' where possible.
ngrinkevich and others added 4 commits June 29, 2019 12:25
This contract is not used by the Keep client, CLI for relay requests has
been implemented with truffle scripts and located in `contract/scripts`.
- Set relayRequestTimeout to 17
- Modified sign(..) function by adding require(..) to handle timeout
- Added tests
@dimpar
Copy link
Contributor Author

dimpar commented Jul 2, 2019

TODO: Need to merge master after upgradable contract components is merged.
This is the actual change for the relay request timeout minus package.json

@dimpar
Copy link
Contributor Author

dimpar commented Jul 2, 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]
[...]

ngrinkevich and others added 16 commits July 3, 2019 08:13
We have decided to query operator contracts instead to avoid duplicated
storage.
Setting new group size should only be possible in the event of contract
upgrade.
Using <noun><action> naming for consistency.
…ents

Upgradeable contract components implementation

Implements RFC5 upgradeable contract components scheme. Adds minor refactoring to reduce the amount of repeated code.

Further additions required to fully match RFC5, saving these for a separate PR:

createGroup() method on Operator contract
Service contract to select operator on requestRelayEntry()
- Set relayRequestTimeout to 17
- Modified sign(..) function by adding require(..) to handle timeout
- Added tests
@dimpar dimpar closed this Jul 4, 2019
@dimpar dimpar deleted the relay-request-timeout branch July 4, 2019 15:28
@Shadowfiend
Copy link
Contributor

👋 would be useful to capture why we closed this PR :)

@dimpar
Copy link
Contributor Author

dimpar commented Jul 10, 2019

The relay request timeout is a relatively small change. Because of Nik's changes, my code was sort of dependent on his work and I created this branch out of his. However, looking now at the history and trying to navigate to my actual changes is a bit confusing. I created a new branch from master (when Nik's changes were merged) for the timeout relay request, it's here. Way easier to look at and review.

@pdyraga
Copy link
Member

pdyraga commented Jul 10, 2019

Couldn't we rebase to master after merging Nik's PR? 🤔

@dimpar
Copy link
Contributor Author

dimpar commented Jul 10, 2019

I tried.. but along the way had also some conflicts and just more headache. It was easier to create a new branch.

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.

4 participants