Add contract and module events #269

DOBEN · 2024-10-15T14:58:51Z

Purpose

Addresses #253

Changes

Add contractEvents fields.
Add logic to decode input_parameter/message if an embedded smart contract schema exists.
Add logic to decode contract logs if an embedded smart contract schema exists.

Victor-N-Suadicani

Various stylistic comments and suggestions, nothing blocking.

backend-rust/migrations/0001_initialize.up.sql

backend-rust/src/graphql_api.rs

Victor-N-Suadicani · 2024-10-31T09:28:06Z

backend-rust/src/indexer.rs

+struct PreparedEventData {
+    /// The prepared event. Note: this is optional temporarily until we can
+    /// handle all events.
+    event:   Option<PreparedEvent>,
+    /// The status of the event (if the event belongs to a successful or
+    /// rejected transaction).
+    success: bool,
+}


This data structure seems very strange to me.

Why not use Result<PreparedEvent, E> with some appropriate E instead of this separate success field? Can you have event: Some(e) at the same time as success: false? What about event: None and success: true? Maybe the success field should be a field on the prepared event instead of in this struct?

Victor-N-Suadicani · 2024-10-31T09:30:57Z

backend-rust/src/indexer.rs

+            PreparedEvent::ModuleDeployed(event) =>
+            // Only save the module into the `modules` table if the transaction was
+            // successful.
+            {


Suggested change

PreparedEvent::ModuleDeployed(event) =>

// Only save the module into the `modules` table if the transaction was

// successful.

{

PreparedEvent::ModuleDeployed(event) => {

// Only save the module into the `modules` table if the transaction was

// successful.

Victor-N-Suadicani · 2024-10-31T09:31:15Z

backend-rust/src/indexer.rs

+            PreparedEvent::ContractInitialized(event) =>
+            // Only save the contract into the `contracts` table if the transaction was
+            // successful.
+            {


Suggested change

PreparedEvent::ContractInitialized(event) =>

// Only save the contract into the `contracts` table if the transaction was

// successful.

{

PreparedEvent::ContractInitialized(event) => {

// Only save the contract into the `contracts` table if the transaction was

// successful.

limemloh · 2024-10-31T07:54:17Z

backend-rust/migrations/0001_initialize.up.sql

+    -- TODO: It would be nice to have every event in a separate row in a table to easily query a specific event from a specific transaction.
    events
        JSONB,


Comment: I am not sure that I agree to the TODO comment above. Depends a bit on our needs, and the comment here does not provide enough information to convince me 😄

There are drawbacks to having events in a separate table and for the queries that I have seen so far, this approach seems best.
PostgreSQL have pretty good support for JSON and since each transaction will have a very limited number of events, searching through all of one transaction should be fast.
But searching through all events of all transactions would probably not work out, but I don't see a need for that.

limemloh · 2024-10-31T08:19:59Z

backend-rust/migrations/0001_initialize.up.sql

@@ -309,11 +310,47 @@ CREATE TABLE contracts(
    init_transaction_index
        BIGINT
        NOT NULL,
+    -- The version of the smart contract.
+    version


Suggestion: I think this version belongs on the module and not the contract.

limemloh · 2024-10-31T08:31:50Z

backend-rust/src/indexer.rs

+    event:   Option<PreparedEvent>,
+    /// The status of the event (if the event belongs to a successful or
+    /// rejected transaction).
+    success: bool,


Request change: I don't think having this boolean is the best representation of this, since a rejected transaction would not have the same information as a successful one. Instead, we should expand the PreparedEvent enum with variants representing only the rejected cases that we care about.

limemloh · 2024-10-31T08:42:40Z

backend-rust/src/indexer.rs

            PreparedEvent::AccountCreation(event) => event.save(tx).await,
+            // TODO: need to handle `rejected` baker events properly.


Comment: I don't think we do.

This event represents the changes we need to do for bakers, a rejected transaction does not change anything on the state of the baker and will not result in this event

limemloh · 2024-10-31T08:49:57Z

backend-rust/src/indexer.rs

            PreparedEvent::BakerEvents(events) => {
                for event in events {
                    event.save(tx).await?;
                }
                Ok(())
            }
-            PreparedEvent::ModuleDeployed(event) => event.save(tx).await,
-            PreparedEvent::ContractInitialized(event) => event.save(tx).await,
+            PreparedEvent::ModuleDeployed(event) =>


As mentioned above, I don't think we should have the event represent both a successful and rejected case.
We only need to update state for the successful case, and this event is only constructed in the successful case at the moment, since AccountTransactionEffects::None above represents all the rejected transactions

limemloh · 2024-10-31T09:53:18Z

backend-rust/src/graphql_api.rs

+                                    trace_element_index,
+                                    contract_events.block_height AS event_block_height,
+                                    transactions.hash as transaction_hash,
+                                    transactions.events,


Suggestion: You should be able to extract the only relevant event like this:

Suggested change

transactions.events,

transactions.events[trace_element_index::INTEGER],

limemloh · 2024-10-31T10:30:32Z

backend-rust/src/graphql_api.rs

+fn decode_value_with_schema(
+    opt_schema: &Option<Type>,
+    schema_name: &str,
+    value: &Vec<u8>,


Suggested change

value: &Vec<u8>,

value: &[u8],

limemloh · 2024-10-31T10:35:45Z

backend-rust/src/graphql_api.rs

+                    amount:            i64::try_from(data.amount.micro_ccd)?,
+                    init_name:         data.init_name.to_string(),
+                    version:           data.contract_version.into(),
+                    // TODO: the send message/input parameter is missing and not exposed by the


Suggestion: Since this is not part of the current API, it would be a new feature and involve frontend work, so could you make it a feature request in an issue instead of this comment. The information is exposed by the SDK, just not very conveniently.

I think the smart contract page will need some work in general after the backend is ready, so it would be good to have a collection of ideas.

limemloh · 2024-10-31T10:49:12Z

backend-rust/migrations/0001_initialize.up.sql

+-- Every successful event associated to a contract.
+CREATE TABLE contract_events (
+    -- An index/id for this event (row number).
+    index


Question: Do we ever use this index?

limemloh · 2024-10-31T10:58:31Z

backend-rust/src/graphql_api.rs

+                                LIMIT $3
+                                OFFSET $4


I agree, we will probably need to do this differently, ideally we would check this with some data and ensure that we actually are improving.

An alternative could be adding a column i, which is an index of the event for this contract, meaning skip x would result in WHERE x > i instead of the offset.

DOBEN changed the base branch from main to rust-backend-modules October 15, 2024 14:59

DOBEN force-pushed the rust-backend-modules branch from b758d16 to 32edfec Compare October 21, 2024 10:45

Base automatically changed from rust-backend-modules to rust-backend October 21, 2024 17:31

DOBEN force-pushed the add-contract-module-events branch from 8f8eecc to 7eaf605 Compare October 22, 2024 06:58

Base automatically changed from rust-backend to main October 24, 2024 09:51

DOBEN force-pushed the add-contract-module-events branch from 686bcb2 to 9f4e397 Compare October 28, 2024 08:26

DOBEN added 15 commits October 30, 2024 20:08

Add database schema

648bb9b

Handle rejected events correctly

ef33bf1

Add contract event to database

f640243

Add contract event query

4e5d9a0

Insert data to contract_events table

c21c391

Add more data to contract_events table

d552036

Add contract events

886f083

Add event and message endpoints

262cf1b

Add log decoding

48534bd

Add contract_logs decoding for init transactions

ba7cbcf

Adjust skip and take values

e3d0b2f

Add pagination info

55cbe78

Add input parameter decoding

25bd49b

Reduce code duplication

162ad35

Update .sqlx

8ea3d7e

DOBEN force-pushed the add-contract-module-events branch from de04c9f to 8ea3d7e Compare October 30, 2024 13:18

Clean up

8f04c5a

DOBEN requested review from Victor-N-Suadicani and limemloh October 30, 2024 17:15

Victor-N-Suadicani approved these changes Oct 31, 2024

View reviewed changes

limemloh reviewed Oct 31, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add contract and module events #269

Add contract and module events #269

DOBEN commented Oct 15, 2024 •

edited

Loading

Victor-N-Suadicani left a comment

Victor-N-Suadicani Oct 31, 2024

Victor-N-Suadicani Oct 31, 2024

Victor-N-Suadicani Oct 31, 2024

limemloh Oct 31, 2024

limemloh Oct 31, 2024

limemloh Oct 31, 2024

limemloh Oct 31, 2024

limemloh Oct 31, 2024

limemloh Oct 31, 2024

limemloh Oct 31, 2024

limemloh Oct 31, 2024

limemloh Oct 31, 2024

limemloh Oct 31, 2024

		PreparedEvent::AccountCreation(event) => event.save(tx).await,
		// TODO: need to handle `rejected` baker events properly.

	transactions.events,
	transactions.events[trace_element_index::INTEGER],

Add contract and module events #269

Are you sure you want to change the base?

Add contract and module events #269

Conversation

DOBEN commented Oct 15, 2024 • edited Loading

Purpose

Changes

Victor-N-Suadicani 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

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

DOBEN commented Oct 15, 2024 •

edited

Loading