Skip to content

interop: supervisor_dependencySet API #684

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

protolambda
Copy link
Contributor

@protolambda protolambda requested a review from ajsutton April 24, 2025 21:04
@protolambda protolambda requested review from a team as code owners April 24, 2025 21:04
- `chainIndex`: `ChainIndex`: the unique short identifier of this chain.
- `activationTime`: `uint64`: when the chain becomes part of the dependency set.
This is the minimum timestamp of the inclusion of an executing message.
- `historyMinTime`: `uint64`: the lower bound of data to store.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- `historyMinTime`: `uint64`: the lower bound of data to store.
- `historyMinTime`: `uint64`: the lower bound of duration to store data.

This is the minimum timestamp of the inclusion of an executing message.
- `historyMinTime`: `uint64`: the lower bound of data to store.
This is the minimum timestamp of an initiating message to be accessible to others.
This is set to 0 when all data since genesis is executable.
Copy link
Member

Choose a reason for hiding this comment

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

assume we will never want to store no data at all then

to determine which chains are available at any particular block.
The returned configuration may change with new chain upgrades.

Parameters: (none)
Copy link
Member

Choose a reason for hiding this comment

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

I would have instinctively though this should take a block number as parameter ..

@emhane emhane added H-interop Hardfork: change planned for Interop upgrade A-supervisor Area: supervisor A-rpc Area: RPC API labels Apr 25, 2025
@emhane emhane added the C-enhancement Category: new feature request label Apr 25, 2025

A unique short identifier for a chain, within the dependency set.

#### `DependencySetConfig`
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we exposing the config format here? We are exposing implementation details of the db (chain index + history min time) as well as the message expiry override time? What would the most useful API be from first principles?

I would guess its something like:

{
  timestamp: { expiryWindow: x, chains: [chainid, chainid] },
  timestamp2: { expiryWindow: y, chains: [chainid, chainid, chainid] }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR is old. The dependency set approach has changed a lot since. I will likely just replace it with an endpoint to get the list of chain IDs in the dependency set.

@emhane
Copy link
Member

emhane commented May 29, 2025

@protolambda
Copy link
Contributor Author

For readers: this PR is not accurate, the format changed last 2 weeks. Will need to update this after other higher priority work is out of the way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Area: RPC API A-supervisor Area: supervisor C-enhancement Category: new feature request H-interop Hardfork: change planned for Interop upgrade
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants