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(core): Add nonce to program id generation #3010

Merged
merged 62 commits into from
Sep 24, 2023
Merged

refactor(core): Add nonce to program id generation #3010

merged 62 commits into from
Sep 24, 2023

Conversation

mqxf
Copy link
Contributor

@mqxf mqxf commented Aug 2, 2023 •
edited by techraed
Loading

Resolves #2821

@gear-tech/dev

@mqxf mqxf requested review from breathx and techraed August 2, 2023 09:18
techraed
techraed requested changes Aug 2, 2023
View reviewed changes
Copy link
Member

@techraed techraed left a comment

Choose a reason for hiding this comment

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

What about changes in core-processor when create_program is called? or upload_program? There are lots of dependant changes like if you change program id generation, then you must change InitPacket instantiation and all places where it's used.

core/src/message/init.rs Outdated Show resolved Hide resolved
breathx
breathx requested changes Aug 2, 2023
View reviewed changes
core/src/ids.rs Outdated Show resolved Hide resolved
core/src/ids.rs Outdated Show resolved Hide resolved
gcli/tests/common/mod.rs Show resolved Hide resolved
gtest/src/program.rs Outdated Show resolved Hide resolved
@breathx breathx changed the title Add nonce to program id generation refactor(core): Add nonce to program id generation Aug 2, 2023
@breathx
Copy link
Member

breathx commented Aug 2, 2023

Once already addressed comments solved, do not forget to replace no-op block with panic in journal handler's program creation on gear pallet

@breathx breathx added the A3-gotissues PR occurred to have issues after the review label Aug 2, 2023
@mqxf mqxf requested review from techraed and breathx August 3, 2023 11:49
techraed
techraed requested changes Aug 3, 2023
View reviewed changes
core/src/ids.rs Outdated Show resolved Hide resolved
core-errors/src/lib.rs Outdated Show resolved Hide resolved
core/src/message/init.rs Outdated Show resolved Hide resolved
NikVolf
NikVolf reviewed Aug 4, 2023
View reviewed changes
core/src/ids.rs Outdated Show resolved Hide resolved
@breathx
Copy link
Member

breathx commented Aug 6, 2023

Resolve finished conversations, fix CI and re-request reviews please @mqxf

@mqxf
Copy link
Contributor Author

mqxf commented Aug 6, 2023

Will do closer to evening today. I am on multiple flights today.

@mqxf mqxf requested a review from techraed August 7, 2023 11:58
techraed
techraed requested changes Aug 18, 2023
View reviewed changes
Copy link
Member

@techraed techraed left a comment

Choose a reason for hiding this comment

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

Please use the next approach:

  • program ids from pallet - hash(concat(message_id, code_id, salt))
  • program ids from wasm program - hash(concat(message_id, code_id, salt, store.initialized.len()))

core/src/ids.rs Outdated Show resolved Hide resolved
core/src/message/context.rs Outdated Show resolved Hide resolved
core/src/message/context.rs Outdated Show resolved Hide resolved
core/src/message/context.rs Outdated Show resolved Hide resolved
ark0f
ark0f previously requested changes Aug 20, 2023
View reviewed changes
core-errors/src/lib.rs Outdated Show resolved Hide resolved
breathx
breathx requested changes Sep 18, 2023
View reviewed changes
core/src/ids.rs Outdated Show resolved Hide resolved
core/src/ids.rs Outdated Show resolved Hide resolved
core/src/message/init.rs Show resolved Hide resolved
core/src/message/init.rs Outdated Show resolved Hide resolved
pallets/gear/src/tests.rs Outdated Show resolved Hide resolved
@mqxf
Copy link
Contributor Author

mqxf commented Sep 21, 2023

#3320 once host macro is removed

@breathx breathx merged commit d76be59 into master Sep 24, 2023
@breathx breathx deleted the ms/issue-2821 branch September 24, 2023 11:55
@shamilsan shamilsan added the B1-releasenotes The feature deserves to be added to the Release Notes label Sep 26, 2023
@osipov-mit osipov-mit mentioned this pull request Sep 29, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NikVolf NikVolf NikVolf left review comments

@breathx breathx breathx approved these changes

@techraed techraed techraed approved these changes

@ark0f ark0f Awaiting requested review from ark0f

Assignees
No one assigned
Labels
A0-pleasereview PR is ready to be reviewed by the team B1-releasenotes The feature deserves to be added to the Release Notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Handle case when gr_create_program results in already existing program
6 participants
@mqxf @breathx @NikVolf @ark0f @techraed @shamilsan