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

chore: HIP-755 - add post implementation updates for accuracy #1098

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

lukelee-sl
Copy link
Member

Description:
Update several details in HIP-755 to better reflect the actual implementation.

@lukelee-sl lukelee-sl self-assigned this Jan 14, 2025
@lukelee-sl lukelee-sl requested a review from mgarbs as a code owner January 14, 2025 01:26
Copy link

netlify bot commented Jan 14, 2025

Deploy Preview for hedera-hips ready!

Name Link
🔨 Latest commit 51906a5
🔍 Latest deploy log https://app.netlify.com/sites/hedera-hips/deploys/6786a6c3af17c10008ffef93
😎 Deploy Preview https://deploy-preview-1098--hedera-hips.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Signed-off-by: Luke Lee <[email protected]>
@david-bakin-sl david-bakin-sl changed the title chore: add post implementation updates for accuracy chore: HIP-755 - add post implementation updates for accuracy Jan 17, 2025
Copy link
Member

@david-bakin-sl david-bakin-sl left a comment

Choose a reason for hiding this comment

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

LGTM (but with some suggestions if you're already doing another revision)

This supports the prior acquisition of signatures and their future submission, as often utilized in direct HAPI transaction submissions.
In order to validate the signatures in the signature map for the `signSchedule(address, bytes)` function call,
a message has to be agreed upon. The most logical message would be the concatenated values of the shard, realm and number of the schedule transaction ID and this value will be used
by convention.

To support safe and easy direct calls by an EOA in accordance with the security model, new facade methods will be added as part of this HRC 755.
Copy link
Member

Choose a reason for hiding this comment

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

This is the only use of "HRC" in this file. Is it supposed to be "HIP"? Or if "HRC" then also spell out the acronym somewhere (footnote)?

Comment on lines +62 to +64
In order to validate the signatures in the signature map for the `signSchedule(address, bytes)` function call,
a message has to be agreed upon. The most logical message would be the concatenated values of the shard, realm and number of the schedule transaction ID and this value will be used
by convention.
Copy link
Member

Choose a reason for hiding this comment

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

These last two sentences are less "specification" (the section name) and more "suggestion to developers". Especially the last sentence. I don't suggest moving them elsewhere, but maybe they should be subordinate in some way, e.g., a bullet point?

(This is me ratholing on style again, sorry.)

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