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

Add transaction's content verification #736

Merged
merged 5 commits into from
Jan 5, 2023

Conversation

samuelmanzanera
Copy link
Member

Description

This PR improves data validation for transfer & hosting tx
For the hosting, this is based on the changes from archethic-foundation/aeweb-cli#87

Fixes #409

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@samuelmanzanera samuelmanzanera added feature New feature request mining Involve transaction validation and mining labels Dec 2, 2022
@samuelmanzanera samuelmanzanera force-pushed the better_transaction_type_validation branch from 92f4670 to 2aa7a1b Compare December 2, 2022 09:37
@bchamagne bchamagne self-requested a review December 6, 2022 14:06
Copy link
Member

@bchamagne bchamagne left a comment

Choose a reason for hiding this comment

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

It does not work because it tries to validate the data transaction with the same schema.
AEWebCLI's data transaction format did not change and looks like this:

%{
  "dir1" => %{
    "file10.txt" => "H4sIAAAAAAAAA0vLzElVMDTgAgALt2T5CAAAAA",
    "file11.txt" => "H4sIAAAAAAAAA0vLzElVMDTkAgBKhn_gCAAAAA"
  },
  "dir2" => %{
    "hello.txt" => "H4sIAAAAAAAAA_NIzcnJ1yupKFHQyM_LqVRIy8xJVcjMU0jJLDLS5AIAt5R0vB4AAAA"
  },
  "dir3" => %{
    "index.html" => "H4sIAAAAAAAAA7PJMLRz8QwyVsjMS0mt0Msoyc2x0QeKcQEAL-DoARkAAAA"
  },
  "file1.txt" => "H4sIAAAAAAAAA0vLzElVMFTgAgBapaazCAAAAA",
  "file2.txt" => "H4sIAAAAAAAAA0vLzElVMOICAMGeIz8HAAAA",
  "file3.txt" => "H4sIAAAAAAAAA0vLzElVMOYCAICvOCYHAAAA"
}

2022-12-06 14:07:05.340 [debug] Invalid AEWeb format [{"Required properties aewebVersion, metaData were not present.", "#"}]

@samuelmanzanera , I'll deal with this

@apoorv-2204
Copy link
Contributor

apoorv-2204 commented Dec 6, 2022

It does not work because it tries to validate the data transaction with the same schema. AEWebCLI's data transaction format did not change and looks like this:

%{
  "dir1" => %{
    "file10.txt" => "H4sIAAAAAAAAA0vLzElVMDTgAgALt2T5CAAAAA",
    "file11.txt" => "H4sIAAAAAAAAA0vLzElVMDTkAgBKhn_gCAAAAA"
  },
  "dir2" => %{
    "hello.txt" => "H4sIAAAAAAAAA_NIzcnJ1yupKFHQyM_LqVRIy8xJVcjMU0jJLDLS5AIAt5R0vB4AAAA"
  },
  "dir3" => %{
    "index.html" => "H4sIAAAAAAAAA7PJMLRz8QwyVsjMS0mt0Msoyc2x0QeKcQEAL-DoARkAAAA"
  },
  "file1.txt" => "H4sIAAAAAAAAA0vLzElVMFTgAgBapaazCAAAAA",
  "file2.txt" => "H4sIAAAAAAAAA0vLzElVMOICAMGeIz8HAAAA",
  "file3.txt" => "H4sIAAAAAAAAA0vLzElVMOYCAICvOCYHAAAA"
}

2022-12-06 14:07:05.340 [debug] Invalid AEWeb format [{"Required properties aewebVersion, metaData were not present.", "#"}]

@samuelmanzanera , I'll deal with this

we need to use branch on this pr
#742

and we need to use aeweb cli branch from this pr
archethic-foundation/aeweb-cli#87

@bchamagne
Copy link
Member

The issue was because the json schema was created for this data transaction format:

%{
  "dir1/file10.txt" => "H4sIAAAAAAAAA0vLzElVMDTgAgALt2T5CAAAAA",
  "dir1/file11.txt" => "H4sIAAAAAAAAA0vLzElVMDTkAgBKhn_gCAAAAA",
  "dir2/hello.txt" => "H4sIAAAAAAAAA_NIzcnJ1yupKFHQyM_LqVRIy8xJVcjMU0jJLDLS5AIAt5R0vB4AAAA",
  "dir3/index.html" => "H4sIAAAAAAAAA7PJMLRz8QwyVsjMS0mt0Msoyc2x0QeKcQEAL-DoARkAAAA",
  "file1.txt" => "H4sIAAAAAAAAA0vLzElVMFTgAgBapaazCAAAAA",
  "file2.txt" => "H4sIAAAAAAAAA0vLzElVMOICAMGeIz8HAAAA",
  "file3.txt" => "H4sIAAAAAAAAA0vLzElVMOYCAICvOCYHAAAA"
}

This format has been discussed but it is not implemented like this yet. Current format is:

%{
  "dir1" => %{
    "file10.txt" => "H4sIAAAAAAAAA0vLzElVMDTgAgALt2T5CAAAAA",
    "file11.txt" => "H4sIAAAAAAAAA0vLzElVMDTkAgBKhn_gCAAAAA"
  },
  "dir2" => %{
    "hello.txt" => "H4sIAAAAAAAAA_NIzcnJ1yupKFHQyM_LqVRIy8xJVcjMU0jJLDLS5AIAt5R0vB4AAAA"
  },
  "dir3" => %{
    "index.html" => "H4sIAAAAAAAAA7PJMLRz8QwyVsjMS0mt0Msoyc2x0QeKcQEAL-DoARkAAAA"
  },
  "file1.txt" => "H4sIAAAAAAAAA0vLzElVMFTgAgBapaazCAAAAA",
  "file2.txt" => "H4sIAAAAAAAAA0vLzElVMOICAMGeIz8HAAAA",
  "file3.txt" => "H4sIAAAAAAAAA0vLzElVMOYCAICvOCYHAAAA"
}

Here's my suggestion to fix with current data transaction format: jsonschema.patch.tar.gz

@apoorv-2204
Copy link
Contributor

The issue was because the json schema was created for this data transaction format:

%{
  "dir1/file10.txt" => "H4sIAAAAAAAAA0vLzElVMDTgAgALt2T5CAAAAA",
  "dir1/file11.txt" => "H4sIAAAAAAAAA0vLzElVMDTkAgBKhn_gCAAAAA",
  "dir2/hello.txt" => "H4sIAAAAAAAAA_NIzcnJ1yupKFHQyM_LqVRIy8xJVcjMU0jJLDLS5AIAt5R0vB4AAAA",
  "dir3/index.html" => "H4sIAAAAAAAAA7PJMLRz8QwyVsjMS0mt0Msoyc2x0QeKcQEAL-DoARkAAAA",
  "file1.txt" => "H4sIAAAAAAAAA0vLzElVMFTgAgBapaazCAAAAA",
  "file2.txt" => "H4sIAAAAAAAAA0vLzElVMOICAMGeIz8HAAAA",
  "file3.txt" => "H4sIAAAAAAAAA0vLzElVMOYCAICvOCYHAAAA"
}

This format has been discussed but it is not implemented like this yet. Current format is:

%{
  "dir1" => %{
    "file10.txt" => "H4sIAAAAAAAAA0vLzElVMDTgAgALt2T5CAAAAA",
    "file11.txt" => "H4sIAAAAAAAAA0vLzElVMDTkAgBKhn_gCAAAAA"
  },
  "dir2" => %{
    "hello.txt" => "H4sIAAAAAAAAA_NIzcnJ1yupKFHQyM_LqVRIy8xJVcjMU0jJLDLS5AIAt5R0vB4AAAA"
  },
  "dir3" => %{
    "index.html" => "H4sIAAAAAAAAA7PJMLRz8QwyVsjMS0mt0Msoyc2x0QeKcQEAL-DoARkAAAA"
  },
  "file1.txt" => "H4sIAAAAAAAAA0vLzElVMFTgAgBapaazCAAAAA",
  "file2.txt" => "H4sIAAAAAAAAA0vLzElVMOICAMGeIz8HAAAA",
  "file3.txt" => "H4sIAAAAAAAAA0vLzElVMOYCAICvOCYHAAAA"
}

Here's my suggestion to fix with current data transaction format: jsonschema.patch.tar.gz

we will proceed with nested json only.

@apoorv-2204
Copy link
Contributor

apoorv-2204 commented Dec 6, 2022

archethic-foundation/aeweb-cli#87
This pr doesnt impl key value pair.It uses nested json struct with metaData props

@bchamagne bchamagne force-pushed the better_transaction_type_validation branch from 2aa7a1b to 1cdc2fe Compare January 3, 2023 17:02
@bchamagne bchamagne self-requested a review January 3, 2023 17:03
@apoorv-2204 apoorv-2204 self-requested a review January 3, 2023 17:04
Copy link
Member

@bchamagne bchamagne left a comment

Choose a reason for hiding this comment

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

Hello, I did the quick fix after the big merge of all AEWeb PRs.
This is ready to merge !

Tested with AEWEB CLI

@bchamagne bchamagne added web hosting issue regarding web hosting and removed mining Involve transaction validation and mining labels Jan 3, 2023
@samuelmanzanera
Copy link
Member Author

Hello, I did the quick fix after the big merge of all AEWeb PRs. This is ready to merge !

Tested with AEWEB CLI

Thanks 👍

@samuelmanzanera samuelmanzanera merged commit c6bbb87 into develop Jan 5, 2023
@samuelmanzanera samuelmanzanera deleted the better_transaction_type_validation branch January 5, 2023 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core team Assigned to the core team feature New feature request web hosting issue regarding web hosting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide a better validation for typed transaction
4 participants