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

getDhtOpType, getDhtOpAction, getDhtOpEntry, getDhtOpSignature functions are incorrect in v0.18.0-rc #306

Closed
guillemcordoba opened this issue Dec 16, 2024 · 5 comments

Comments

@guillemcordoba
Copy link
Contributor

In holochain v0.4, the DhtOp type was changed to:

export type DhtOp =
  | {
      ChainOp: ChainOp;
    }
  | { WarrantOp: WarrantOp };

Which actually makes all the DhtOp helpers incorrect, these is how they look like:

export function getDhtOpEntry(op: DhtOp): Entry | undefined {
  return Object.values(op)[0][2];
}

So they don't throw a typescript error, but actually they won't work. They should probably accept a ChainOp argument instead of a DhtOp one.

@matthme
Copy link
Contributor

matthme commented Dec 17, 2024

It might still be useful not to have to filter by WarrantOp and ChainOp before passing it in? What about a type DhtOpSubType that can be either of the ChainOp types or Warrant?

@guillemcordoba
Copy link
Contributor Author

Yeah some of them yeah, some of them no. WarrantOp only carries the ActionHash for the invalid record, which makes things like getDhtOpType() or getDhtOpAction() not work at all with it.

@matthme
Copy link
Contributor

matthme commented Dec 17, 2024

I was thinking that getDhtOpType would now return the following type:

export enum ChainOpSubType {
  StoreRecord = "StoreRecord",
  StoreEntry = "StoreEntry",
  RegisterAgentActivity = "RegisterAgentActivity",
  RegisterUpdatedContent = "RegisterUpdatedContent",
  RegisterUpdatedRecord = "RegisterUpdatedRecord",
  RegisterDeletedBy = "RegisterDeletedBy",
  RegisterDeletedEntryAction = "RegisterDeletedEntryAction",
  RegisterAddLink = "RegisterAddLink",
  RegisterRemoveLink = "RegisterRemoveLink",
  Warrant = "Warrant",
}

And getDhtOpAction could return undefined in the warrant case.

But maybe it's cleaner to just call it getChainOpType and getChainOpAction and take a ChainOp as you suggest and then the caller needs to filter explicityl beforehand.

@matthme
Copy link
Contributor

matthme commented Dec 17, 2024

Here's a PR: #308
What do you think @guillemcordoba? (Can't add you as reviewer in the PR itself apparently)

@matthme
Copy link
Contributor

matthme commented Dec 17, 2024

Fixed with #308 and published as 0.18.0-rc.2. But unfortunately I didn't see the other issue about the wrong ChainIntegrityWarrant type so this will be part of a next release...

@matthme matthme closed this as completed Dec 17, 2024
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

No branches or pull requests

2 participants