search

Yuri-SVB / Great_Wall

Protocol and application for providing Kerckhoffian, 0-trust, deviceless coercion-resistance in self-custody.
MIT License
24 stars 14 forks source link

[ISSUE #44] Feature save state #63

Closed fredericomba closed 7 months ago

fredericomba commented 7 months ago

Pull request (not complete, ongoing)

fredericomba commented 7 months ago

Also, you introduced a solution to bug that has been addressed before

No, I didn't. The "one-click many-events" bug is not fixed by this pull request.

Are there any other blockers that are preventing this pull request from being merged?

MuhammadMuradG commented 7 months ago

Good, but before we start in detailed review you need to address my last point that I mentioned, in simple words you need to remove tests for now.

If you are interested, you may need to open a new PR about implementing tests for our application. This PR/PRs should include, description of how we can implement the test and the strategy on how we are going.

fredericomba commented 7 months ago

The tests have been removed.

Yuri-SVB commented 7 months ago

@MuhammadMuradG and @fredericomba:

The use case of irregular tree topology is not only possible, but rather probable.

Indexing nodes with single integer makes sense in regular topology because the computation of integer to node and node to integer is straightforward, but this is not the case if topology is irregular.

Unless there is established, consecrated doctrine for indexation of tree nodes with integers in opposition to integer list (possibly strings consisting of comma-separated integers) representing path to node starting from root, specifically in trees of irregular topology, we should have our indexes only consist of path to node: either array or string, whatever comes simpler, because having integer + this array would therefore be an unnecessary moving part.

Consider that testing, auditing an integer-based indexation would have to first check the correctness of index-to-path and path-to-index mapping.

If nobody points out such doctrine, let's refactor towards path-based indexation.

fredericomba commented 7 months ago

If nobody points out such doctrine, let's refactor towards path-based indexation.

The code now uses strings representing a path of choices (integers separated by comma).

MuhammadMuradG commented 7 months ago

Thanks for you effort but we decided after discussion to close this as we have simple implementation for this feature at #79. Please, comment on #79 if you address a problem in implementation...