-
Notifications
You must be signed in to change notification settings - Fork 19
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
Fix RPC timeouts #76
Fix RPC timeouts #76
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,6 +61,7 @@ | |
creator: "${creator}", | ||
fpmm_: { | ||
answerFinalizedTimestamp_not: null, | ||
creationTimestamp_gt: "${from_timestamp}", | ||
isPendingArbitration: false | ||
Comment on lines
63
to
65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We only get trades |
||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,9 @@ | |
from packages.valory.skills.market_manager_abci.rounds import SynchronizedData | ||
|
||
|
||
DAY_UNIX = 24 * 60 * 60 | ||
|
||
|
||
def to_content(query: str) -> bytes: | ||
"""Convert the given query string to payload content, i.e., add it under a `queries` key and convert it to bytes.""" | ||
finalized_query = {"query": query} | ||
|
@@ -184,7 +187,9 @@ def _fetch_redeem_info(self) -> Generator[None, None, Optional[list]]: | |
self._fetch_status = FetchStatus.IN_PROGRESS | ||
|
||
safe = self.synchronized_data.safe_contract_address | ||
query = trades.substitute(creator=safe.lower()) | ||
redeem_margin = self.params.redeem_margin_days * DAY_UNIX | ||
from_timestamp = self.synced_time - redeem_margin | ||
query = trades.substitute(creator=safe.lower(), from_timestamp=from_timestamp) | ||
Comment on lines
-187
to
+192
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume the event filter later on is made s.t. the lowest block is used from this response, correct? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeap, exactly. |
||
|
||
# workaround because we cannot have multiple response keys for a single `ApiSpec` | ||
res_key_backup = self.current_subgraph.response_info.response_key | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,6 +38,9 @@ | |
BenchmarkTool = BaseBenchmarkTool | ||
|
||
|
||
GNOSIS_RPC_TIMEOUT_DAYS = 25 | ||
|
||
|
||
class SharedState(BaseSharedState): | ||
"""Keep the current shared state of the skill.""" | ||
|
||
|
@@ -72,8 +75,40 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: | |
self.languages: List[str] = self._ensure("languages", kwargs, List[str]) | ||
self.average_block_time: int = self._ensure("average_block_time", kwargs, int) | ||
self.abt_error_mult: int = self._ensure("abt_error_mult", kwargs, int) | ||
self._redeem_margin_days: int = 0 | ||
self.redeem_margin_days = self._ensure("redeem_margin_days", kwargs, int) | ||
super().__init__(*args, **kwargs) | ||
|
||
@property | ||
def redeem_margin_days(self) -> int: | ||
"""Get the margin in days of the redeeming information.""" | ||
return self._redeem_margin_days | ||
|
||
@redeem_margin_days.setter | ||
def redeem_margin_days(self, redeem_margin_days: int) -> None: | ||
"""Get the margin in days of the redeeming information.""" | ||
value_enforcement = ( | ||
f"The value needs to be in the exclusive range (0, {GNOSIS_RPC_TIMEOUT_DAYS}) " | ||
f"and manual redeeming has to be performed for markets older than {GNOSIS_RPC_TIMEOUT_DAYS - 1} days." | ||
) | ||
|
||
if redeem_margin_days <= 0: | ||
raise ValueError( | ||
"The margin in days for the redeeming information (`redeem_margin_days`) " | ||
f"cannot be set to {redeem_margin_days} <= 0. {value_enforcement}" | ||
) | ||
if redeem_margin_days >= GNOSIS_RPC_TIMEOUT_DAYS: | ||
raise ValueError( | ||
"Due to a constraint of the Gnosis RPCs, it is not possible to configure the redeeming " | ||
f"information's time window to exceed {GNOSIS_RPC_TIMEOUT_DAYS - 1} days " | ||
f"(currently {redeem_margin_days=}). To clarify, these RPCs experience timeouts " | ||
"when attempting to filter for historical on-chain events. " | ||
"Practical testing of the service has revealed that timeouts consistently occur for blocks " | ||
f"approximately {GNOSIS_RPC_TIMEOUT_DAYS} days old. {value_enforcement}" | ||
) | ||
Comment on lines
+90
to
+108
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not allow the margin to be less than 1 or more than |
||
|
||
self._redeem_margin_days = redeem_margin_days | ||
|
||
@property | ||
def creators_iterator(self) -> Iterator[Tuple[str, List[str]]]: | ||
"""Return an iterator of market per creators.""" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Advice to the user in case a timeout occurs.