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

Encrypt metadata spec and types #4176

Merged
merged 9 commits into from
Nov 16, 2023

Conversation

paweljakubas
Copy link
Contributor

@paweljakubas paweljakubas commented Oct 23, 2023

  • Adding spec and change swagger accordingly
  • Add field encrypt_metadata to ApiConstructTransactionPostData
  • Introduce ApiDecodeTransactionPostData`
  • deal with all code adjustment
  • adjust unit tests

Issue Number

ADP-322

@paweljakubas paweljakubas self-assigned this Oct 23, 2023
@paweljakubas paweljakubas force-pushed the paweljakubas/adp-322/encrypt-metadata branch from f1fa5af to c2a7360 Compare October 26, 2023 14:04
@paweljakubas paweljakubas force-pushed the paweljakubas/adp-322/encrypt-metadata branch 3 times, most recently from 33489e9 to 8ef1343 Compare November 3, 2023 15:59
@paweljakubas paweljakubas force-pushed the paweljakubas/adp-322/encrypt-metadata branch 2 times, most recently from c4b5cf1 to bd2a689 Compare November 9, 2023 15:02
@paweljakubas paweljakubas mentioned this pull request Nov 9, 2023
4 tasks
@paweljakubas paweljakubas force-pushed the paweljakubas/adp-322/encrypt-metadata branch 2 times, most recently from ad2601c to 8b08f75 Compare November 14, 2023 12:16
```
{
...
"metadata": "metadata encrypted"
Copy link
Contributor

@Unisay Unisay Nov 15, 2023

Choose a reason for hiding this comment

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

If "encrypt_metadata" is an object for extensibility purposes (an object allows to specify an encryption algorithm that was chosen), would it also make sense to make the encrypted metadata an object carrying the information about selected encryption algorithm?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when we support other encryption algos this is going to be a must. but for now I made sure encrypt_metadata object could be extended in non-intrusive way for future. When we add it then we will add "string" with algo details along encrypted bytes. We will easily be able to handle this in noninvasive way and be backward compatible. thanks for the remark!

Comment on lines +79 to +82
{ "1": "Hard times create strong men."
, "2": "Strong men create good times."
, "3": "Good times create weak men."
, "4": "And, weak men create hard times."
Copy link
Contributor

Choose a reason for hiding this comment

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

💎

...
}
```
as metadata values have 64-byte limit. In that case the encrypted metadata is encoded in the successive bytes.
Copy link
Contributor

Choose a reason for hiding this comment

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

So this makes it impossible to decrypt a single key without using the cardano-wallet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we need to append 64-byte chunks and then you can decrypt whole. And you can do it outside cardano-wallet if you comply with algos and passphrase symmetrizing algo

-- Above is the way to get a compiler error when number of fields changes,
-- in order not to forget to update the pattern below:
case body of
ApiConstructTransactionData
{ payments = Nothing
, withdrawal = Nothing
, metadata = Nothing
, encryptMetadata = Nothing
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

@paweljakubas paweljakubas force-pushed the paweljakubas/adp-322/encrypt-metadata branch from 8b08f75 to c42044d Compare November 16, 2023 09:58
@paweljakubas paweljakubas added this pull request to the merge queue Nov 16, 2023
Merged via the queue into master with commit fd9eb91 Nov 16, 2023
3 checks passed
@paweljakubas paweljakubas deleted the paweljakubas/adp-322/encrypt-metadata branch November 16, 2023 11:17
WilliamKingNoel-Bot pushed a commit that referenced this pull request Nov 16, 2023
…llet points the work accomplished in this PR. Before you submit, don't forget to: CODE-OF-CONDUCT.md CONTRIBUTING.md LICENSE MAINTAINERS.md README.md cabal.project default.nix docker-compose.yml docs flake.lock flake.nix floskell.json fourmolu.yaml hie-direnv.yaml justfile lib nix prototypes reports scripts shell.nix specifications test touch.me.CI weeder.dhall Make sure the GitHub PR fields are correct: ✓ Set a good Title for your PR. ✓ Assign yourself to the PR. ✓ Assign one or more reviewer(s). ✓ Link to a Jira issue, and/or other GitHub issues or PRs. ✓ In the PR description delete any empty sections and all text commented in <!--, so that this text does not appear in merge commit messages. CODE-OF-CONDUCT.md CONTRIBUTING.md LICENSE MAINTAINERS.md README.md cabal.project default.nix docker-compose.yml docs flake.lock flake.nix floskell.json fourmolu.yaml hie-direnv.yaml justfile lib nix prototypes reports scripts shell.nix specifications test touch.me.CI weeder.dhall Don't waste reviewers' time: ✓ If it's a draft, select the Create Draft PR option. ✓ Self-review your changes to make sure nothing unexpected slipped through. CODE-OF-CONDUCT.md CONTRIBUTING.md LICENSE MAINTAINERS.md README.md cabal.project default.nix docker-compose.yml docs flake.lock flake.nix floskell.json fourmolu.yaml hie-direnv.yaml justfile lib nix prototypes reports scripts shell.nix specifications test touch.me.CI weeder.dhall Try to make your intent clear: ✓ Write a good Description that explains what this PR is meant to do. ✓ Jira will detect and link to this PR once created, but you can also link this PR in the description of the corresponding Jira ticket. ✓ Highlight what Testing you have done. ✓ Acknowledge any changes required to the Documentation. --> - [x] Adding spec and change swagger accordingly - [x] Add field `encrypt_metadata` to `ApiConstructTransactionPostData` - [x] Introduce ApiDecodeTransactionPostData` - [x] deal with all code adjustment - [x] adjust unit tests ### Issue Number ADP-322 Source commit: fd9eb91
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