Skip to content

Commit d76289f

Browse files
authored
Merge pull request #2207 from eqlabs/krisztian/0.14.2-backport-bugfixes
chore: backport some fixes to 0.14.2
2 parents c0137d6 + 36f44a7 commit d76289f

File tree

8 files changed

+129
-28
lines changed

8 files changed

+129
-28
lines changed

CHANGELOG.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,18 @@ More expansive patch notes and explanations may be found in the specific [pathfi
77
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
88
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
99

10+
## Unreleased
11+
12+
### Fixed
13+
14+
- Pathfinder sometimes returns an INVALID_CONTINUATION_TOKEN error when requesting events from the pending block and providing a continuation token.
15+
- `starknet_getEvents` incorrectly returns pending events if `from_block` is greater than latest_block_number + 1.
16+
- `starknet_getEvents` incorrectly does not return pending events if `from_block` is `pending` and `to_block` is missing.
17+
18+
### Added
19+
20+
- `--sync.l1-poll-interval` CLI option has been added to set the poll interval for L1 state. Defaults to 30s.
21+
1022
## [0.14.1] - 2024-07-29
1123

1224
### Fixed

crates/gateway-client/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ httpmock = { workspace = true }
3838
lazy_static = { workspace = true }
3939
pathfinder-crypto = { path = "../crypto" }
4040
pretty_assertions_sorted = { workspace = true }
41+
reqwest = { workspace = true, features = ["json"] }
4142
starknet-gateway-test-fixtures = { path = "../gateway-test-fixtures" }
4243
test-log = { workspace = true, features = ["trace"] }
4344
tracing-subscriber = { workspace = true }

crates/gateway-client/src/builder.rs

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -449,24 +449,26 @@ fn retry_condition(e: &SequencerError) -> bool {
449449

450450
match e {
451451
SequencerError::ReqwestError(e) => {
452-
if e.is_body() || e.is_connect() || e.is_timeout() {
452+
if e.is_timeout() {
453+
info!(reason=%e, "Request failed, retrying. Fetching the response or parts of it timed out. Try increasing request timeout by using the `--gateway.request-timeout` CLI option.");
454+
return true;
455+
}
456+
457+
if e.is_body() || e.is_connect() {
453458
info!(reason=%e, "Request failed, retrying");
454459
} else if e.is_status() {
455-
match e.status() {
456-
Some(
457-
StatusCode::NOT_FOUND
458-
| StatusCode::TOO_MANY_REQUESTS
459-
| StatusCode::BAD_GATEWAY
460-
| StatusCode::SERVICE_UNAVAILABLE
461-
| StatusCode::GATEWAY_TIMEOUT,
462-
) => {
460+
match e.status().expect("status related error") {
461+
StatusCode::NOT_FOUND
462+
| StatusCode::TOO_MANY_REQUESTS
463+
| StatusCode::BAD_GATEWAY
464+
| StatusCode::SERVICE_UNAVAILABLE
465+
| StatusCode::GATEWAY_TIMEOUT => {
463466
debug!(reason=%e, "Request failed, retrying");
464467
}
465-
Some(StatusCode::INTERNAL_SERVER_ERROR) => {
468+
StatusCode::INTERNAL_SERVER_ERROR => {
466469
error!(reason=%e, "Request failed, retrying");
467470
}
468-
Some(_) => warn!(reason=%e, "Request failed, retrying"),
469-
None => unreachable!(),
471+
_ => warn!(reason=%e, "Request failed, retrying"),
470472
}
471473
} else if e.is_decode() {
472474
error!(reason=%e, "Request failed, retrying");

crates/pathfinder/src/bin/pathfinder/config.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,14 @@ Examples:
132132
)]
133133
poll_interval: std::num::NonZeroU64,
134134

135+
#[arg(
136+
long = "sync.l1-poll-interval",
137+
long_help = "L1 state poll interval in seconds",
138+
default_value = "30",
139+
env = "PATHFINDER_L1_POLL_INTERVAL_SECONDS"
140+
)]
141+
l1_poll_interval: std::num::NonZeroU64,
142+
135143
#[arg(
136144
long = "color",
137145
long_help = "This flag controls when to use colors in the output logs.",
@@ -669,6 +677,7 @@ pub struct Config {
669677
pub sqlite_wal: JournalMode,
670678
pub max_rpc_connections: std::num::NonZeroUsize,
671679
pub poll_interval: std::time::Duration,
680+
pub l1_poll_interval: std::time::Duration,
672681
pub color: Color,
673682
pub p2p: P2PConfig,
674683
pub debug: DebugConfig,
@@ -953,6 +962,7 @@ impl Config {
953962
},
954963
max_rpc_connections: cli.max_rpc_connections,
955964
poll_interval: Duration::from_secs(cli.poll_interval.get()),
965+
l1_poll_interval: Duration::from_secs(cli.l1_poll_interval.get()),
956966
color: cli.color,
957967
p2p: P2PConfig::parse_or_exit(cli.p2p),
958968
debug: DebugConfig::parse(cli.debug),

crates/pathfinder/src/bin/pathfinder/main.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,12 @@ Hint: This is usually caused by exceeding the file descriptor limit of your syst
174174
});
175175
let execution_storage = storage_manager
176176
.create_read_only_pool(execution_storage_pool_size)
177-
.context(r"")?;
177+
.context(
178+
r"Creating database connection pool for execution
179+
180+
Hint: This is usually caused by exceeding the file descriptor limit of your system.
181+
Try increasing the file limit to using `ulimit` or similar tooling.",
182+
)?;
178183

179184
let p2p_storage = storage_manager
180185
.create_pool(NonZeroU32::new(1).unwrap())
@@ -534,6 +539,7 @@ fn start_sync(
534539
gossiper: state::Gossiper,
535540
gateway_public_key: pathfinder_common::PublicKey,
536541
_p2p_client: Option<p2p::client::peer_agnostic::Client>,
542+
_verify_tree_hashes: bool,
537543
) -> tokio::task::JoinHandle<anyhow::Result<()>> {
538544
start_feeder_gateway_sync(
539545
storage,
@@ -569,6 +575,7 @@ fn start_feeder_gateway_sync(
569575
sequencer: pathfinder_context.gateway,
570576
state: sync_state.clone(),
571577
head_poll_interval: config.poll_interval,
578+
l1_poll_interval: config.l1_poll_interval,
572579
pending_data: tx_pending,
573580
block_validation_mode: state::l2::BlockValidationMode::Strict,
574581
websocket_txs,

crates/pathfinder/src/state/sync.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ pub struct SyncContext<G, E> {
7676
pub sequencer: G,
7777
pub state: Arc<SyncState>,
7878
pub head_poll_interval: Duration,
79+
pub l1_poll_interval: Duration,
7980
pub pending_data: WatchSender<PendingData>,
8081
pub block_validation_mode: l2::BlockValidationMode,
8182
pub websocket_txs: Option<TopicBroadcasters>,
@@ -95,7 +96,7 @@ where
9596
ethereum: value.ethereum.clone(),
9697
chain: value.chain,
9798
core_address: value.core_address,
98-
poll_interval: value.head_poll_interval,
99+
poll_interval: value.l1_poll_interval,
99100
}
100101
}
101102
}
@@ -181,6 +182,7 @@ where
181182
sequencer,
182183
state,
183184
head_poll_interval,
185+
l1_poll_interval: _,
184186
pending_data,
185187
block_validation_mode: _,
186188
websocket_txs,

crates/rpc/src/method/get_events.rs

Lines changed: 68 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,12 @@ pub async fn get_events(
8787
// The database query for 3 and 4 is combined into one step.
8888
//
8989
// 4 requires some additional logic to handle some edge cases:
90-
// a) Query database
91-
// b) if full page -> return page
90+
// a) if from_block_number > pending_block_number -> return empty result
91+
// b) Query database
92+
// c) if full page -> return page
9293
// check if there are matching events in the pending block
9394
// and return a continuation token for the pending block
94-
// c) else if empty / partially full -> append events from start of pending
95+
// d) else if empty / partially full -> append events from start of pending
9596
// if there are more pending events return a continuation token
9697
// with the appropriate offset within the pending block
9798

@@ -134,27 +135,43 @@ pub async fn get_events(
134135
.transaction()
135136
.context("Creating database transaction")?;
136137

137-
// Handle the trivial (1) and (2) cases.
138+
// Handle the trivial (1), (2) and (4a) cases.
138139
match (&request.from_block, &request.to_block) {
139-
(Some(Pending), non_pending) if *non_pending != Some(Pending) => {
140+
(Some(Pending), id) if !matches!(id, Some(Pending) | None) => {
140141
return Ok(types::GetEventsResult {
141142
events: Vec::new(),
142143
continuation_token: None,
143144
});
144145
}
145-
(Some(Pending), Some(Pending)) => {
146+
(Some(Pending), Some(Pending) | None) => {
146147
let pending = context
147148
.pending_data
148149
.get(&transaction)
149150
.context("Querying pending data")?;
150151
return get_pending_events(&request, &pending, continuation_token);
151152
}
153+
(Some(BlockId::Number(from_block)), Some(BlockId::Pending)) => {
154+
let pending = context
155+
.pending_data
156+
.get(&transaction)
157+
.context("Querying pending data")?;
158+
159+
// `from_block` is larger than or equal to pending block's number
160+
if from_block >= &pending.number {
161+
return Ok(types::GetEventsResult {
162+
events: Vec::new(),
163+
continuation_token: None,
164+
});
165+
}
166+
}
152167
_ => {}
153168
}
154169

155170
let from_block = map_from_block_to_number(&transaction, request.from_block)?;
156171
let to_block = map_to_block_to_number(&transaction, request.to_block)?;
157172

173+
// Handle cases (3) and (4) where `from_block` is non-pending.
174+
158175
let (from_block, requested_offset) = match continuation_token {
159176
Some(token) => token.start_block_and_offset(from_block)?,
160177
None => (from_block, 0),
@@ -460,10 +477,11 @@ impl std::fmt::Display for ContinuationToken {
460477

461478
impl ContinuationToken {
462479
fn offset_in_block(&self, block_number: BlockNumber) -> Result<usize, GetEventsError> {
463-
if self.block_number == block_number {
464-
Ok(self.offset)
465-
} else {
466-
Err(GetEventsError::InvalidContinuationToken)
480+
use std::cmp::Ordering;
481+
match Ord::cmp(&self.block_number, &block_number) {
482+
Ordering::Equal => Ok(self.offset),
483+
Ordering::Less => Ok(0),
484+
Ordering::Greater => Err(GetEventsError::InvalidContinuationToken),
467485
}
468486
}
469487

@@ -959,6 +977,14 @@ mod tests {
959977
assert_eq!(result.events, &all[3..4]);
960978
assert_eq!(result.continuation_token, None);
961979

980+
// continuing from a page that does exist, should return all events (even from
981+
// pending)
982+
input.filter.chunk_size = 123;
983+
input.filter.continuation_token = Some("0-0".to_string());
984+
let result = get_events(context.clone(), input.clone()).await.unwrap();
985+
assert_eq!(result.events, all);
986+
assert_eq!(result.continuation_token, None);
987+
962988
// nonexistent page: offset too large
963989
input.filter.chunk_size = 123; // Does not matter
964990
input.filter.continuation_token = Some("3-3".to_string()); // Points to after the last event
@@ -1039,5 +1065,37 @@ mod tests {
10391065
.events;
10401066
assert_eq!(events, &all[1..2]);
10411067
}
1068+
1069+
#[tokio::test]
1070+
async fn from_block_past_pending() {
1071+
let context = RpcContext::for_tests_with_pending().await;
1072+
1073+
let input = GetEventsInput {
1074+
filter: EventFilter {
1075+
from_block: Some(BlockId::Number(BlockNumber::new_or_panic(4))),
1076+
to_block: Some(BlockId::Pending),
1077+
chunk_size: 100,
1078+
..Default::default()
1079+
},
1080+
};
1081+
let result = get_events(context, input).await.unwrap();
1082+
assert!(result.events.is_empty());
1083+
}
1084+
1085+
#[tokio::test]
1086+
async fn from_block_pending_to_block_none() {
1087+
let context = RpcContext::for_tests_with_pending().await;
1088+
1089+
let input = GetEventsInput {
1090+
filter: EventFilter {
1091+
from_block: Some(BlockId::Pending),
1092+
to_block: None,
1093+
chunk_size: 100,
1094+
..Default::default()
1095+
},
1096+
};
1097+
let result = get_events(context, input).await.unwrap();
1098+
assert!(!result.events.is_empty());
1099+
}
10421100
}
10431101
}

crates/rpc/src/v03/method/get_events.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -460,10 +460,11 @@ impl std::fmt::Display for ContinuationToken {
460460

461461
impl ContinuationToken {
462462
fn offset_in_block(&self, block_number: BlockNumber) -> Result<usize, GetEventsError> {
463-
if self.block_number == block_number {
464-
Ok(self.offset)
465-
} else {
466-
Err(GetEventsError::InvalidContinuationToken)
463+
use std::cmp::Ordering;
464+
match Ord::cmp(&self.block_number, &block_number) {
465+
Ordering::Equal => Ok(self.offset),
466+
Ordering::Less => Ok(0),
467+
Ordering::Greater => Err(GetEventsError::InvalidContinuationToken),
467468
}
468469
}
469470

@@ -959,6 +960,14 @@ mod tests {
959960
assert_eq!(result.events, &all[3..4]);
960961
assert_eq!(result.continuation_token, None);
961962

963+
// continuing from a page that does exist, should return all events (even from
964+
// pending)
965+
input.filter.chunk_size = 123;
966+
input.filter.continuation_token = Some("0-0".to_string());
967+
let result = get_events(context.clone(), input.clone()).await.unwrap();
968+
assert_eq!(result.events, all);
969+
assert_eq!(result.continuation_token, None);
970+
962971
// nonexistent page: offset too large
963972
input.filter.chunk_size = 123; // Does not matter
964973
input.filter.continuation_token = Some("3-3".to_string()); // Points to after the last event

0 commit comments

Comments
 (0)