Use service to query ethereum node in Ethereum Tracker example #3144

jvff · 2025-01-17T18:42:43Z

Motivation

While working on PR #2631, the end-to-end test for the Ethereum Tracker example started failing. That happened because the updated HTTP request API also included the response headers in the oracle recording, and a "Date" header made the response non-deterministic.

Proposal

Use the service to perform the HTTP request, and only send back to the contract the actual deterministic information that matters.

In order to do so, the EthereumClient had to be split in two, so that it could use either the contract system APIs or the service system APIs.

Test Plan

The end-to-end test passes locally in my repository when this fix is applied to the changes in PR #2631.

Release Plan

Nothing to do / These changes follow the usual release cycle, as it only changes the example and the SDK.

Links

reviewer checklist

Prepare to add custom queries beyond the ones derived from `SimpleObject`, in a way that has access to the `EthereumTrackerService` type.

Read the Ethereum event and return a processed response.

Have one to use in contracts and one to use in services.

Make it easier to convert to the custom type.

Avoid non-deterministic HTTP responses.

Read the Ethereum events and return a processed response.

Avoid non-deterministic HTTP responses.

It's no longer used by the contract because now it's the service that communicates with the Ethereum node.

Improve the interface when it becomes possible.

MathieuDutSik · 2025-01-18T16:45:29Z

examples/ethereum-tracker/src/contract.rs

+        let async_graphql::Value::Object(data_object) = response.data else {
+            panic!("Unexpected response from `readTransferEvents`: {response:#?}");
+        };
+        let async_graphql::Value::List(ref events) = data_object["readTransferEvents"] else {


Isn't the "ref variable" something that we want to avoid?

MathieuDutSik · 2025-01-19T20:37:03Z

examples/ethereum-tracker/src/contract.rs

+        let request = async_graphql::Request::new(format!(
+            r#"query {{ readTransferEvents(endBlock: {end_block}) }}"#
+        ));


The code in main is reading events from a start_block to a to_block (renamed end_block here).
In the new code, there isn't anymore a start_code. How do we avoid repeating the operations?

The service has access to the state, too, and read_transfer_events uses the start_block field. No need to pass it in here.

MathieuDutSik · 2025-01-19T20:39:25Z

examples/ethereum-tracker/src/contract.rs

+            let value = U256::from_str_radix(&value_string[2..], 16)
+                .expect("Balance could not be parsed as a hex string");


It is a little unfortunate to have this rather low level conversion occurring in this part of the code.

I'd imagine that FromStr would work on the whole string here, and interpret it as hexadecimal because it starts with 0x? I.e. maybe we can just use value_string.parse::<U256>()?

MathieuDutSik · 2025-01-19T20:50:53Z

examples/ethereum-tracker/src/service.rs

+    async fn read_transfer_events(&self, end_block: u64) -> Vec<TransferEvent> {
+        let start_block = *self.0.state.start_block.get();
+        let events = self
+            .read_events(
+                "Transfer(address indexed,address indexed,uint256)",
+                start_block,
+                end_block,
+            )
+            .await;


What I find a little odd is that now the code has two ways to access the events.
Here, we have the read_transfer_events and in fn update a different code.

But update is using read_transfer_events. How else could we move this functionality into the service and use it from the contract?

MathieuDutSik · 2025-01-19T21:14:16Z

examples/ethereum-tracker/src/contract.rs

+        self.state
+            .save()
+            .await
+            .expect("Failed to write updated storage");


What is exactly the justification for adding this self.state.save() here? This is the only smart contract where a self.state.save() is added to the fn instantiate function. Why is that so?
For example, the crowd-funding and fungible are doing meaningful operations in their fn instantiate functions. Why haven't they some self.state.save().
Side question 1: When is the fn store function used?
Side question 2: Now, the fn save function is doing the writing to the batch and if the batch is empty, then nothing is done. So, could the save could be added by the linera-rpc?

afck · 2025-01-20T09:29:01Z

examples/ethereum-tracker/src/contract.rs

+        let application_id = self.runtime.application_id();
+        let response = self.runtime.query_service(application_id, request);
+
+        let async_graphql::Value::Object(data_object) = response.data else {


We could avoid all those async_graphql::Value::Variant matches by using response.data.into_json() instead. The JSON Value type has a few more convenient methods like as_str etc. (And we'd rarely need as_object because e.g. json_data["readInitialEvent"]["address"] works, too.)

(Anyway, maybe that's outside the scope of this PR.)

afck · 2025-01-20T09:41:35Z

examples/ethereum-tracker/src/contract.rs

+            let value = U256::from_str_radix(&value_string[2..], 16)
+                .expect("Balance could not be parsed as a hex string");


I'd imagine that FromStr would work on the whole string here, and interpret it as hexadecimal because it starts with 0x? I.e. maybe we can just use value_string.parse::<U256>()?

afck · 2025-01-20T09:46:44Z

examples/ethereum-tracker/src/service.rs

+    async fn read_transfer_events(&self, end_block: u64) -> Vec<TransferEvent> {
+        let start_block = *self.0.state.start_block.get();
+        let events = self
+            .read_events(
+                "Transfer(address indexed,address indexed,uint256)",
+                start_block,
+                end_block,
+            )
+            .await;


But update is using read_transfer_events. How else could we move this functionality into the service and use it from the contract?

afck · 2025-01-20T09:47:39Z

examples/ethereum-tracker/src/contract.rs

+        let request = async_graphql::Request::new(format!(
+            r#"query {{ readTransferEvents(endBlock: {end_block}) }}"#
+        ));


The service has access to the state, too, and read_transfer_events uses the start_block field. No need to pass it in here.

afck · 2025-01-20T09:49:38Z

examples/ethereum-tracker/src/service.rs

+        events
+            .into_iter()
+            .map(|event| {
+                let EthereumDataType::Address(source) = event.values[0].clone() else {


We aren't using event anymore after this, so we might as well use event.values.into_iter() instead of cloning the values.

afck · 2025-01-20T09:52:33Z

examples/ethereum-tracker/src/state.rs

@@ -5,7 +5,7 @@ use ethereum_tracker::U256Cont;
 use linera_sdk::views::{linera_views, MapView, RegisterView, RootView, ViewStorageContext};

 /// The application state.
-#[derive(RootView, async_graphql::SimpleObject)]
+#[derive(RootView)]


Alternatively, we could use ComplexObject and remove the manual implementations of the four getters.

afck · 2025-01-20T09:53:47Z

linera-sdk/src/ethereum.rs

-use crate::contract::wit::contract_system_api;
+use crate::{contract::wit::contract_system_api, service::wit::service_system_api};
+
+// TODO(#3143): Unify the two types into a single `EthereumClient` type.


Does the Contract one still make sense at all? With the planned change for the HTTP oracle, I imagine it wouldn't work anymore?

jvff added 9 commits January 17, 2025 18:09

Expand GraphQL query implementation

fa81c13

Prepare to add custom queries beyond the ones derived from `SimpleObject`, in a way that has access to the `EthereumTrackerService` type.

Implement reading initial event in the service

17f5223

Read the Ethereum event and return a processed response.

Split EthereumClient type in two

8d74420

Have one to use in contracts and one to use in services.

Implement From<U256> for U256Cont

3e4c256

Make it easier to convert to the custom type.

Make contract use service to read initial event

2f89e73

Avoid non-deterministic HTTP responses.

Implement reading transfer events in the service

682acaf

Read the Ethereum events and return a processed response.

Make contract use service to read transfer events

782e8fa

Avoid non-deterministic HTTP responses.

Remove obsoleted get_endpoints method

24f85af

It's no longer used by the contract because now it's the service that communicates with the Ethereum node.

Add a TODO for reunifying EthereumClient

bc4de02

Improve the interface when it becomes possible.

jvff added the enhancement New feature or request label Jan 17, 2025

jvff requested review from ma2bd, MathieuDutSik, afck, deuszx and bart-linera January 17, 2025 18:42

jvff self-assigned this Jan 17, 2025

MathieuDutSik reviewed Jan 18, 2025

View reviewed changes

MathieuDutSik reviewed Jan 19, 2025

View reviewed changes

afck reviewed Jan 20, 2025

View reviewed changes

gabu1096 approved these changes Jan 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use service to query ethereum node in Ethereum Tracker example #3144

Use service to query ethereum node in Ethereum Tracker example #3144

jvff commented Jan 17, 2025

MathieuDutSik Jan 18, 2025

MathieuDutSik Jan 19, 2025

afck Jan 20, 2025

MathieuDutSik Jan 19, 2025

afck Jan 20, 2025

MathieuDutSik Jan 19, 2025

afck Jan 20, 2025

MathieuDutSik Jan 19, 2025

afck Jan 20, 2025

afck Jan 20, 2025

afck Jan 20, 2025

afck Jan 20, 2025

afck Jan 20, 2025

afck Jan 20, 2025

afck Jan 20, 2025

		let value = U256::from_str_radix(&value_string[2..], 16)
		.expect("Balance could not be parsed as a hex string");

Use service to query ethereum node in Ethereum Tracker example #3144

Are you sure you want to change the base?

Use service to query ethereum node in Ethereum Tracker example #3144

Conversation

jvff commented Jan 17, 2025

Motivation

Proposal

Test Plan

Release Plan

Links

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment