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

feat: fvm: add support for looking up past tipset CIDs #9687

Merged
merged 2 commits into from
Nov 22, 2022

Conversation

Stebalien
Copy link
Member

We do this by adding yet another "getter" to the VM that resolves an epoch into a TipSetKey.

Alternatively, we could stick this functionality on Rand, but then it wouldn't really be "rand" but something else...

Depends on filecoin-project/filecoin-ffi#339

Merge plan:

  1. Merge feat: plumb tipset CIDs filecoin-ffi#339.
  2. Rebase this PR on master.
  3. Merge this PR to master.
  4. Port this change to the nv18 branches.

Unfortunately, it needs to land on master because it bubbles a breaking change in the FFI (where the FVM now needs the ability to lookup tipset CIDs).

Alternatively, we could make this optional with a runtime type assertion. That is, if we find that we're using "v3" inside the FFI, we could type-assert that the "externs" supports the TipsetCid method. That would let us merge the FFI changes without making any changes to the interface.

@Stebalien Stebalien requested a review from a team as a code owner November 18, 2022 22:04
Copy link
Contributor

@arajasek arajasek left a comment

Choose a reason for hiding this comment

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

Yeah, this is how I would do it too.

@Stebalien Stebalien force-pushed the feat/fevm-tipset-cid branch from 302308a to 93c2c10 Compare November 19, 2022 20:13
@Stebalien Stebalien changed the base branch from feat/nv18-fevm to master November 19, 2022 20:13
@Stebalien
Copy link
Member Author

@arajasek I've rebased on master, merge when ready.

Note: this updates the FFI. Technically, we can get away with not doing that on master if you're so inclined.

@Stebalien
Copy link
Member Author

Actually, I'm just going to revert that part.

@Stebalien Stebalien force-pushed the feat/fevm-tipset-cid branch 2 times, most recently from f577166 to 2e5cfb2 Compare November 19, 2022 20:23
@@ -98,6 +99,28 @@ func (k *TipSetKey) UnmarshalJSON(b []byte) error {
return nil
}

func (k TipSetKey) Cid() (cid.Cid, error) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@arajasek cherry-picked from feat/nv18-fevm. Please review.

ychiaoli18 and others added 2 commits November 22, 2022 10:52
(cherry-picked from feat/nv18-fevm)
We do this by adding yet another "getter" to the VM that resolves an
epoch into a TipSetKey.
@arajasek arajasek force-pushed the feat/fevm-tipset-cid branch from 2e5cfb2 to 2bc1abc Compare November 22, 2022 15:59
@jennijuju jennijuju merged commit e78d130 into master Nov 22, 2022
@jennijuju jennijuju deleted the feat/fevm-tipset-cid branch November 22, 2022 17:41
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.

4 participants