feat: gateway #252

jbcaron · 2024-09-09T14:44:47Z

Pull Request type

Feature

What is the current behavior?

Resolves: #186

What is the new behavior?

Does this introduce a breaking change?

No

Other information

github-actions · 2024-09-09T15:05:02Z

Coverage report

The coverage rate is 69.10867865519937%

47% of new lines are covered.

Diff Coverage details (click to unfold)

crates/primitives/gateway/src/receipt.rs

80.0% of new lines are covered

crates/client/gateway/src/client/builder.rs

36.0% of new lines are covered

crates/primitives/receipt/src/into_starknet_core.rs

100.0% of new lines are covered

crates/primitives/receipt/src/lib.rs

83.33333333333334% of new lines are covered

crates/primitives/gateway/src/state_update.rs

100.0% of new lines are covered

crates/client/gateway/src/client/request_builder.rs

63.88888888888889% of new lines are covered

crates/primitives/receipt/src/from_blockifier.rs

0.0% of new lines are covered

crates/node/src/cli/gateway.rs

0.0% of new lines are covered

crates/client/gateway/src/server/worker.rs

0.0% of new lines are covered

crates/client/gateway/src/client/methods.rs

97.95918367346938% of new lines are covered

crates/primitives/gateway/src/transaction.rs

50.0% of new lines are covered

crates/client/gateway/src/server/handler.rs

0.0% of new lines are covered

crates/client/gateway/src/error.rs

0.0% of new lines are covered

crates/primitives/transactions/src/lib.rs

88.23529411764706% of new lines are covered

crates/node/src/main.rs

100.0% of new lines are covered

crates/primitives/gateway/src/block.rs

47.36842105263158% of new lines are covered

crates/primitives/receipt/src/from_starknet_provider.rs

100.0% of new lines are covered

crates/primitives/utils/src/lib.rs

100.0% of new lines are covered

crates/client/gateway/src/server/helpers.rs

0.0% of new lines are covered

crates/primitives/convert/src/hex_serde.rs

100.0% of new lines are covered

crates/client/gateway/src/server/router.rs

0.0% of new lines are covered

crates/node/src/service/gateway.rs

59.25925925925926% of new lines are covered

cchudant

cool!
I think we should move the client to another crate

cchudant · 2024-09-10T14:21:28Z

crates/primitives/gateway/src/block.rs

+
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+#[serde(deny_unknown_fields)]
+pub struct BlockProvider {


why block provider name

cchudant · 2024-09-10T14:24:16Z

crates/primitives/gateway/src/block.rs

+#[serde(deny_unknown_fields)]
+pub enum BlockStatus {
+    Pending,
+    Aborted,


add todo: what does that mean,,??

I think we should:

check if RPC has those / equivalents

if so, ask #starknet-json-rpc slack
otherwise i think we should ask starkware what it is supposed to mean?

cchudant · 2024-09-10T14:37:01Z

crates/tests/src/lib.rs

@@ -185,6 +185,7 @@ impl MadaraCmdBuilder {
                format!("{}", self.tempdir.as_ref().display()),
                "--rpc-port".into(),
                format!("{}", self.port.0),
+                "--".into(),


cchudant · 2024-09-10T14:48:24Z

crates/client/gateway/src/server/handler.rs

+    let state_diff = match backend.get_block_state_diff(&resolved_block_id) {
+        Ok(Some(state_diff)) => state_diff,
+        Ok(None) => {
+            return StarknetError::block_not_found().into();
+        }
+        Err(e) => {
+            log::error!("Error getting contract class hash at: {}", e);
+            return storage_error_to_response(e);
+        }
+    };


example of how to refactor these functions

backend.get_block_state_diff(&resolved_block_id) .or_internal_server_error("Error getting contract class hash")? .ok_or(StarknetError::block_not_found())?

crates/client/gateway/src/server/router.rs

cchudant · 2024-09-10T15:02:37Z

crates/node/src/cli/gateway.rs

+
+    /// The gateway port to listen at.
+    #[arg(long, value_name = "GATEWAY PORT", default_value = "8080")]
+    pub gateway_port: u16,


maybe we want CORS at least here, i don't really know

cchudant · 2024-09-10T15:09:07Z

crates/client/block_import/src/verify_apply.rs

@@ -262,15 +262,15 @@ fn block_hash(
        parent_block_hash,
        block_number,
        global_state_root,
-        sequencer_address,
+        sequencer_address: Some(sequencer_address),


i don't have an opinion on whether we should have that as an option (same for the other 3 fields)

cchudant · 2024-09-10T15:13:27Z

crates/client/gateway/src/server/worker.rs

+        }
+    });
+
+    log::info!("🌐 Gateway endpoint started at {}", listener.local_addr());


if we add cors, match the message from rpc

log::info!( "📱 Running JSON-RPC server at {} (allowed origins={})", local_addr.map_or_else(|| "unknown".to_string(), |a| a.to_string()), format_cors(cors.as_ref()) );

Trantorian1

A few nitpicks, otherwise all seems good 👍

Trantorian1 · 2024-09-20T08:05:40Z

crates/client/gateway/src/client/methods.rs

+    use super::*;
+
+    #[tokio::test]
+    async fn test_get_block() {


Why no assert? It seems you are testing for block 0, maybe we could check the contents of the block too, or at the very least asserts its block number?

Trantorian1 · 2024-09-20T08:06:37Z

crates/client/gateway/src/client/methods.rs

+    }
+
+    #[tokio::test]
+    async fn test_get_state_update() {


Trantorian1 · 2024-09-20T08:14:41Z

crates/client/gateway/src/server/error.rs

+    fn or_internal_server_error<C: fmt::Display>(self, context: C) -> Result<T, GatewayError>;
+}
+
+impl<T, E: Into<GatewayError>> ResultExt<T, E> for Result<T, E> {


Trantorian1 · 2024-09-20T08:20:17Z

crates/client/gateway/src/server/helpers.rs

+    }
+}
+
+// append_pair("includeBlock", "true");


Consider deleting this comment (nitpick)

jbcaron added sequencer The label for sequencing related issues full node Full Node only related issues labels Sep 9, 2024

jbcaron self-assigned this Sep 9, 2024

jbcaron force-pushed the gateway branch 2 times, most recently from ba05390 to 4220b60 Compare September 9, 2024 14:54

jbcaron force-pushed the gateway branch 4 times, most recently from b9b65a8 to 38256b5 Compare September 10, 2024 11:32

cchudant reviewed Sep 10, 2024

View reviewed changes

antiyro marked this pull request as ready for review September 12, 2024 12:08

jbcaron force-pushed the gateway branch 3 times, most recently from b31f96d to bbbf378 Compare September 17, 2024 14:53

jbcaron added 2 commits September 19, 2024 16:19

gateway

36831f0

add add_transaction endpoint

4eb62dc

jbcaron force-pushed the gateway branch from bbbf378 to 4eb62dc Compare September 19, 2024 12:20

Trantorian1 approved these changes Sep 20, 2024

View reviewed changes

jbcaron added 2 commits September 20, 2024 14:18

fix: disable_root

21a715a

fix fmt

d1a03b8

feat: gateway #252

Are you sure you want to change the base?

feat: gateway #252

Conversation

jbcaron commented Sep 9, 2024

Pull Request type

What is the current behavior?

What is the new behavior?

Does this introduce a breaking change?

Other information

github-actions bot commented Sep 9, 2024

Coverage report

crates/primitives/gateway/src/receipt.rs

crates/client/gateway/src/client/builder.rs

crates/primitives/receipt/src/into_starknet_core.rs

crates/primitives/receipt/src/lib.rs

crates/primitives/gateway/src/state_update.rs

crates/client/gateway/src/client/request_builder.rs

crates/primitives/receipt/src/from_blockifier.rs

crates/node/src/cli/gateway.rs

crates/client/gateway/src/server/worker.rs

crates/client/gateway/src/client/methods.rs

crates/primitives/gateway/src/transaction.rs

crates/client/gateway/src/server/handler.rs

crates/client/gateway/src/error.rs

crates/primitives/transactions/src/lib.rs

crates/node/src/main.rs

crates/primitives/gateway/src/block.rs

crates/primitives/receipt/src/from_starknet_provider.rs

crates/primitives/utils/src/lib.rs

crates/client/gateway/src/server/helpers.rs

crates/primitives/convert/src/hex_serde.rs

crates/client/gateway/src/server/router.rs

crates/node/src/service/gateway.rs

cchudant left a comment

Choose a reason for hiding this comment

cchudant Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cchudant Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Trantorian1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cchudant Sep 10, 2024 •

edited

Loading

cchudant Sep 10, 2024 •

edited

Loading