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.

Sign up for GitHub

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

Jump to bottom

refactor: Make domain structure shallow #4734

Merged
merged 1 commit into from
Jun 21, 2024
Merged

refactor: Make domain structure shallow #4734

merged 1 commit into from
Jun 21, 2024

Conversation

SamHSmith
Copy link
Contributor

Description

Linked issue

Closes #{issue_number}

Benefits

Checklist

  • I've read CONTRIBUTING.md
  • I've used the standard signed-off commit format (or will squash just before merging)
  • All applicable CI checks pass (or I promised to make them pass later)
  • (optional) I've written unit tests for the code changes
  • I replied to all comments after code review, marking all implemented changes with thumbs up

@SamHSmith SamHSmith self-assigned this Jun 15, 2024
@SamHSmith SamHSmith changed the title [refactor] 3921: Make domain structure shallow refactor: Make domain structure shallow Jun 15, 2024
@github-actions github-actions bot added the api-changes Changes in the API for client libraries label Jun 15, 2024
@Erigara Erigara self-assigned this Jun 17, 2024
Erigara
Erigara reviewed Jun 17, 2024
View reviewed changes
Copy link
Contributor

@Erigara Erigara left a comment

Choose a reason for hiding this comment

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

Looks good in general, minor suggestions

data_model/src/domain.rs Outdated Show resolved Hide resolved
data_model/src/domain.rs Outdated Show resolved Hide resolved
core/src/smartcontracts/isi/account.rs Outdated Show resolved Hide resolved
@DCNick3
Copy link
Contributor

DCNick3 commented Jun 18, 2024 •
edited
Loading

AFAIU, unregistering domain will leave iroha in a somewhat inconsistent state, where it has orphaned accounts in its world structure. Before that they were cascade-deleted because they were stored inside the domain.

Related: #4672

@SamHSmith SamHSmith force-pushed the shallow_domain branch 3 times, most recently from c52a7d6 to 0c07613 Compare June 21, 2024 00:58
@SamHSmith @mversic
refactor: Make domain structure shallow
391bc25 
Signed-off-by: Sam H. Smith <sam.henning.smith@protonmail.com>
mversic
mversic approved these changes Jun 21, 2024
View reviewed changes
Copy link
Contributor

@mversic mversic left a comment

Choose a reason for hiding this comment

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

I thought we said this change will not affect public API, but it does

core/src/smartcontracts/isi/account.rs Show resolved Hide resolved
core/src/state.rs Show resolved Hide resolved
core/src/state.rs Show resolved Hide resolved
data_model/src/domain.rs Show resolved Hide resolved
@mversic mversic merged commit f018902 into hyperledger:main Jun 21, 2024
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DCNick3 DCNick3 DCNick3 left review comments

@Erigara Erigara Erigara approved these changes

@mversic mversic mversic approved these changes

@s8sato s8sato Awaiting requested review from s8sato s8sato is a code owner

@outoftardis outoftardis Awaiting requested review from outoftardis outoftardis is a code owner

@0x009922 0x009922 Awaiting requested review from 0x009922 0x009922 is a code owner

@VAmuzing VAmuzing Awaiting requested review from VAmuzing

@dima74 dima74 Awaiting requested review from dima74 dima74 is a code owner

Assignees

@DCNick3 DCNick3

@SamHSmith SamHSmith

@Erigara Erigara

Labels
api-changes Changes in the API for client libraries
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@SamHSmith @DCNick3 @Erigara @mversic