feat: add event informant and stream cid (part 4) #484

smrz2001 · 2024-08-15T22:46:16Z

This PR includes the changes for storing the source node DID and stream Init Event CID in the Event Store.

This is needed to be able to filter out events submitted to the C1 node via API so that only our events are anchored.

linear · 2024-08-15T22:46:18Z

AES-301 Update schema to store source of events

recon/src/recon/tests.rs

store/src/sql/entities/event.rs

migrations/sqlite/20240809205107_event_source_anchored.up.sql

event-svc/src/event/store.rs

event-svc/src/store/sql/query.rs

event-svc/src/event/service.rs

stbrody · 2024-09-10T19:24:39Z

event-svc/src/event/service.rs

    /// In the future, we will need to do more event validation (verify all EventID pieces, hashes, signatures, etc).
    pub(crate) async fn parse_discovered_event(
        item: &ReconItem<EventId>,
+        source: Option<String>,


why is source optional? Aren't all events going to have a source?

not when we insert from a migration.
API: self
Generated time event: self
Recon: the NodeId.try_from_peer_id(peer_id)
migration: None

event-svc/src/event/service.rs

stbrody · 2024-09-10T19:30:54Z

event-svc/src/store/sql/entities/event.rs

+        stream_cid: Cid,
        event_cid: Cid,
        event: unvalidated::Event<Ipld>,
+        source: Option<String>,


I see we add stream_cid, source, and anchored to the sql table, but we're only adding the first two here. Does this need to take anchored also?

We just took out anchored. We were planning to use it in part 5 but are not doing so anymore.

in that case do we still need it in the sql table?

I think it was removed, unless "it" is not anchored 😅.. init_cid and informant were added

stbrody · 2024-09-10T19:33:38Z

interest-svc/src/interest/store.rs

    async fn insert_many(
        &self,
        items: &[ReconItem<Self::Key>],
+        // the recon::Store trait is shared between InterestService and EventService but only events track a source.


don't interests have a source too? Could this be unified?

As we discussed in standup, we do not currently have a use case for storing the source of an interest. We can add support for this when needed.

stbrody · 2024-09-10T19:35:05Z

migrations/sqlite/20240809205107_event_source_anchored.up.sql

+-- Add up migration script here
+
+-- The CID of the Init Event for the stream
+ALTER TABLE ceramic_one_event ADD COLUMN init_cid BLOB DEFAULT NULL;


how will this get backfilled for events that were created before this field was present in the db?

Really ideally this column would be non-nullable and always required, but that makes the upgrade path trickier

Right now we're only using this field while generating Time Events, which will only happen for events that also have the informant populated. We should never have events with an informant and without a init_cid. This means that the init_cid being missing is ok from the self-anchoring perspective, but it does still need to be populated so that we can support things like looking up events for a stream.

Created a ticket for this.

stbrody · 2024-09-10T19:37:02Z

one/src/migrations.rs

    let network = opts.network.to_network(&opts.local_network_id)?;
    let db_opts: DBOpts = (&opts).into();
    let sqlite_pool = db_opts.get_sqlite_pool().await?;
+    // the source did:key is none for migrations as we don't know the source node.


hmm, couldn't an argument be made to just consider this local node the source? This is the first node bringing this data into the Recon network after all...

Maybe, maybe not. It was likely created and anchored by someone else before the migration. e.g. the old node you are migrating from.

right, but that's almost certainly "logically" the same node. Ie operated by the same person, serving the same purpose/application, and directly replacing the old node in every way. What's the harm in considering the migrated event as having this node as the source?

If these events are considered originating locally, they might be considered (newly) eligible for self-anchoring.

During migration, it would probably be safe to assume that any events that needed to be anchored were already anchored.

dav1do

A few more questions but overall it's looking good. Happy to get on a call to talk through them if it helps speed it up. And the PR name is out of date now - not sure how to comment on that directly :).

anchor-remote/src/cas_remote.rs

core/src/node_id.rs

recon/src/protocol.rs

migrations/sqlite/20240809205107_event_source_anchored.up.sql

event-svc/src/store/metrics.rs

event-svc/src/event/store.rs

core/src/node_id.rs

event/src/unvalidated/event.rs

stbrody · 2024-09-16T19:50:35Z

migrations/sqlite/20240809205107_event_source_anchored.up.sql

@@ -0,0 +1,9 @@
+-- Add up migration script here


looks like the file names for these migrations probably should be updated in a follow-up since they don't really reflect what is happening in them anymore

e.g. they don't add "anchored" anymore

Yes, we fixed this in part 5.

Oh dang, I didn't notice that. Unfortunately, I don't think you can change a migration name after it's been applied somewhere.. pretty sure it's a unique constraint or part of the hash with how sqlx manages them. We can try it but iirc it will error saying something like migration with name x has previously been applied but is missing.

presumably so long as we fix this before it goes out in a release it's fine?

well, it's in a prerelease now and if anyone has run main locally they'll need to manually adjust their db (or delete the file). plus any test nodes we're running it on... my opinion is that's it a bummer but isn't worth the risk/hassle. I don't really pay attention to migration names though (clearly) and just look at the schemas.

smrz2001 assigned smrz2001 and AaronGoldman Aug 15, 2024

smrz2001 requested review from a team, AaronGoldman, dav1do, nathanielc and stbrody as code owners August 15, 2024 22:46

smrz2001 requested review from JulissaDantes and removed request for a team August 15, 2024 22:46

smrz2001 temporarily deployed to github-tests-2024 August 15, 2024 22:56 — with GitHub Actions Inactive

smrz2001 mentioned this pull request Aug 17, 2024

feat: self-anchoring (part 5) #488

Merged

smrz2001 temporarily deployed to github-tests-2024 August 19, 2024 17:33 — with GitHub Actions Inactive

smrz2001 changed the title ~~feat: add event source, stream cid, and anchored timestamp~~ feat: add event source, stream cid, and anchored timestamp (part 4) Aug 23, 2024

smrz2001 force-pushed the feature/aes-301-update-schema-to-store-source-of-events branch from 0adc56f to e1d6233 Compare August 23, 2024 23:28

smrz2001 changed the base branch from main to feature/aes-216-built-interface-for-performing-and-validing-anchors August 23, 2024 23:29

smrz2001 temporarily deployed to github-tests-2024 August 23, 2024 23:38 — with GitHub Actions Inactive

smrz2001 force-pushed the feature/aes-301-update-schema-to-store-source-of-events branch from e1d6233 to 46a78fe Compare August 23, 2024 23:39

smrz2001 temporarily deployed to github-tests-2024 August 23, 2024 23:49 — with GitHub Actions Inactive

AaronGoldman force-pushed the feature/aes-216-built-interface-for-performing-and-validing-anchors branch 2 times, most recently from 52c49ec to 395e326 Compare August 29, 2024 22:17

AaronGoldman force-pushed the feature/aes-301-update-schema-to-store-source-of-events branch from 46a78fe to d4a0c1f Compare August 29, 2024 23:20

AaronGoldman temporarily deployed to github-tests-2024 August 29, 2024 23:31 — with GitHub Actions Inactive

dav1do reviewed Sep 4, 2024

View reviewed changes

smrz2001 force-pushed the feature/aes-216-built-interface-for-performing-and-validing-anchors branch 3 times, most recently from 1bc7d0c to a466a1e Compare September 6, 2024 22:20

Base automatically changed from feature/aes-216-built-interface-for-performing-and-validing-anchors to main September 6, 2024 23:18

AaronGoldman force-pushed the feature/aes-301-update-schema-to-store-source-of-events branch from d4a0c1f to 30a0e2e Compare September 10, 2024 15:10

AaronGoldman temporarily deployed to github-tests-2024 September 10, 2024 15:21 — with GitHub Actions Inactive

AaronGoldman reviewed Sep 10, 2024

View reviewed changes

event-svc/src/event/store.rs Outdated Show resolved Hide resolved

AaronGoldman reviewed Sep 10, 2024

View reviewed changes

event-svc/src/store/sql/query.rs Outdated Show resolved Hide resolved

AaronGoldman force-pushed the feature/aes-301-update-schema-to-store-source-of-events branch from f04ee57 to ec2c8b4 Compare September 10, 2024 19:32

stbrody reviewed Sep 10, 2024

View reviewed changes

AaronGoldman temporarily deployed to github-tests-2024 September 10, 2024 19:44 — with GitHub Actions Inactive

AaronGoldman temporarily deployed to github-tests-2024 September 11, 2024 06:32 — with GitHub Actions Inactive

smrz2001 temporarily deployed to github-tests-2024 September 12, 2024 00:27 — with GitHub Actions Inactive

smrz2001 requested review from dav1do and stbrody September 12, 2024 15:54

dav1do reviewed Sep 12, 2024

View reviewed changes

event/src/unvalidated/event.rs Outdated Show resolved Hide resolved

smrz2001 changed the title ~~feat: add event source, stream cid, and anchored timestamp (part 4)~~ feat: add event informant, stream cid (part 4) Sep 12, 2024

smrz2001 changed the title ~~feat: add event informant, stream cid (part 4)~~ feat: add event informant and stream cid (part 4) Sep 12, 2024

feat: anchor service

9faee13

smrz2001 force-pushed the feature/aes-301-update-schema-to-store-source-of-events branch from 0a18a1a to 9faee13 Compare September 12, 2024 22:03

smrz2001 requested a review from dav1do September 12, 2024 22:04

smrz2001 enabled auto-merge September 12, 2024 22:04

dav1do approved these changes Sep 12, 2024

View reviewed changes

smrz2001 temporarily deployed to github-tests-2024 September 12, 2024 22:20 — with GitHub Actions Inactive

smrz2001 added this pull request to the merge queue Sep 12, 2024

Merged via the queue into main with commit c03ee02 Sep 12, 2024

smrz2001 deleted the feature/aes-301-update-schema-to-store-source-of-events branch September 12, 2024 23:04

stbrody reviewed Sep 16, 2024

View reviewed changes

smrz2001 mentioned this pull request Sep 18, 2024

chore: version v0.36.0 #533

Merged

feat: add event informant and stream cid (part 4) #484

feat: add event informant and stream cid (part 4) #484

Uh oh!

Conversation

smrz2001 commented Aug 15, 2024

Uh oh!

linear bot commented Aug 15, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smrz2001 Sep 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dav1do left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

smrz2001 Sep 12, 2024 •

edited

Loading