Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build Guide: querying source chains #535

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

pdaoust
Copy link
Collaborator

@pdaoust pdaoust commented Feb 19, 2025

Closes #474 . Talks about it from the perspective of oneself (query), another agent (get_agent_activity), and validation (must_get_agent_activity).

@pdaoust pdaoust marked this pull request as ready for review February 21, 2025 20:06
@pdaoust pdaoust enabled auto-merge (squash) February 21, 2025 20:06
@pdaoust pdaoust requested a review from a team February 21, 2025 20:07
Copy link
Member

@ThetaSinner ThetaSinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The query and validation lookups look good. I'm suggesting keeping coordinator agent activity as a separate topic

use hdk::prelude::*;

#[hdk_extern]
pub fn get_all_movies_i_authored() -> Vec<Record> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how many people agree with me, we shouldn't return records from zome calls in general. I'm not super fussy about that appearing here because it's just demonstrating something simple and we don't need a bunch of type conversion, but I will call that out when I see it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scaffolding currently returns Records from zome calls. An app's UI will generally want the author, action hash, and timestamp along with the entry data.

I see that Record is a cumbersome data structure, but unless we wanted to implement something like https://github.com/holochain-open-dev/infrastructure/blob/15f89a9a2026b4c80b092a55b2faed605f435b91/packages/utils/src/entry-record.ts#L10 in the hdk, and update scaffolding to use it, I'd suggest that returning Records is the best practice for now.

}
```

## Query another agent's source chain
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a really different topic. You don't get back their source chain in the same way you do locally and it's for quite different purposes.

There's a huge downside, in that you have to fetch and cache their entire chain to do this. Plus it's really for examining an agent's state, not the content of their chain. You see what valid/invalid data they have, the top of their chain, any warrants etc. That's not the same as retrieving your own source chain. I think this would be better saved until we're dealing with warrants and blocking.

This function was actually flagged to be renamed in the run-up to the Holo launch to make it clearer what it does.

let chain_with_entries = activity.valid_activity
.iter()
.map(|a| {
let maybe_record = get(a.1, GetOptions::network())?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could issue thousands of gets and cause them all to be cached. This is so expensive, I'd call it an antipattern. Data that this agent intended to share should be discovered through the DHT with links and the like, ideally not via their chain unless you have a really good reason to audit their activity or something.


### Get source chain status summary

`get_agent_activity` is for more than just querying an agent's source chain. It also returns the _status of the agent's source chain_, which includes evidence of [source chain forks](/resources/glossary/#fork-source-chain) and invalid actions.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the primary intention of the function, and why I think it's probably not meant for this page.


Validation imposes an extra constraint on source chain queries. A source chain can grow over time, including branching or forking. That means a source chain, when retrieved by agent public key alone, is a non-deterministic source of data, which you can't use in validation<!-- TODO: link to validation page -->. Instead, you can use [`must_get_agent_activity`](https://docs.rs/hdi/latest/hdi/chain/fn.must_get_agent_activity.html), whose [filter struct](https://docs.rs/holochain_integrity_types/latest/holochain_integrity_types/chain/struct.ChainFilter.html) and return value remove non-determinism.

`must_get_agent_activity` only allows you to select a contiguous, bounded slice of a source chain, and doesn't return any information about the validity of the actions in that slice or the chain as a whole.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here would be a good place to suggest that this is only used within RegisterAgentActivity validation

// Now find out how many times we've tried to update the same `Movie` as
// we're trying to update now.
let times_I_updated_this_movie = movie_update_actions
.fold(1, |c, a| c + if a.original_action_address == action.original_action_address { 1 } else { 0 });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not fold(0?


* <code>sequence_range: <a href="https://docs.rs/holochain_zome_types/latest/holochain_zome_types/query/enum.ChainQueryFilterRange.html">ChainQueryFilterRange</a></code>: A start and end point on the source chain, either:
* `Unbounded`: the beginning and current tip of the chain
* `ActionSeqRange(u32, u32)`: start and end sequence indices, inclusive.
Copy link
Member

@mattyg mattyg Feb 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* `ActionSeqRange(u32, u32)`: start and end sequence indices, inclusive.
* `ActionSeqRange(u32, u32)`: start and end sequence indices, inclusive, zero-based.

Maybe this is not necessary, if you think so just ignore this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Practical guide for using Source Chain Query
3 participants