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

Replace @id w/ f:ledger in transactions #578

Merged
merged 3 commits into from
Sep 25, 2023

Conversation

cap10morgan
Copy link
Contributor

@cap10morgan cap10morgan commented Sep 19, 2023

...for identifying the ledger. This is more semantically correct and fixes an issue with these not necessarily being IRIs which broke credential verification.

Also adds context-type to connection protocol and looks it up from there in transact instead of making it an opt passed separately and redundantly.

Upcoming server & http-api-gateway PRs will adapt them to this change.

Part of https://github.com/fluree/core/issues/32

...for identifying the ledger. This is more semantically correct and fixes an issue with these not necessarily being IRIs which broke credential verification.

Also adds context-type to connection protocol and looks it up from there in transact instead of making it an opt passed separately and redundantly.
@cap10morgan cap10morgan requested a review from a team September 19, 2023 19:30
Returns the new db.

Note: Loading the ledger results in a new ledger object, so references to existing
ledger objects will be rendered stale. To obtain a ledger with the new changes,
call `load` on the ledger alias."
[conn json-ld opts]
[conn json-ld]
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like opts is also used by h-a-g to pass in the :did: https://github.com/fluree/http-api-gateway/blob/main/src/fluree/http_api/handlers/ledger.clj#L81-L85, is that going to be an issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, thanks. I just searched this codebase. I'll add that back in.

Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be nice if someday we enumerated what the possible opts are for our top-level api fns 😄

It's used by HTTP API code
Copy link
Contributor

@mpoffald mpoffald left a comment

Choose a reason for hiding this comment

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

🛸

@cap10morgan
Copy link
Contributor Author

@mpoffald Sorry I added one more commit to get the h-a-g test suite passing (and I think it's more correct overall regardless). Mind taking a quick extra look at that?

@mpoffald
Copy link
Contributor

LGTM!

@cap10morgan cap10morgan merged commit d9f9811 into main Sep 25, 2023
6 checks passed
@cap10morgan cap10morgan deleted the feature/fluree-ledger-key-in-txns branch September 25, 2023 22:12
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.

2 participants