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

Revisit GA contract ga_main_w_temporary.aes #4116

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Revisit GA contract ga_main_w_temporary.aes #4116

wants to merge 5 commits into from

Conversation

velzevur
Copy link
Member

@velzevur velzevur commented Mar 17, 2023 •
edited

Closes #4083.

The work on this PR is supported by Aeternity Crypto Foundation

@velzevur
Copy link
Member Author

We will discuss adding another layer of super accounts that can change the master.

@loxs
Copy link
Contributor

loxs commented Mar 17, 2023

Are we sure that wallets can sign with this ASDF prefix? I thought it needs to be exactly the same as the "message" prefix, which seems to be "aeternity Signed Message:\n" and otherwise, the wallet will refuse to sign it?

@velzevur
Copy link
Member Author

The best case would be dry-running the to_sign entrypoint to generate the hash to be signed for you so you don't generate the hash yourself. The prefix is fed to the contract in the init so it could be any message.

hanssv
hanssv previously approved these changes Mar 23, 2023
View reviewed changes
test/contracts/ga_main_w_temporary.aes Outdated
= Plain
| TimeBound(int)
| NBound(int)

type prefix_t = bytes(19)
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 a bit silly... You may pick any prefix as long as it is 19 bytes 🙈

My suggestion would be to make the prefix fixed for this contract.

@hanssv
Copy link
Member

hanssv commented Mar 24, 2023

What we would really like is an arbitrary sized binary/bytes type in Sophia - let's add that to the wish-list

uwiger
uwiger previously approved these changes Mar 24, 2023
View reviewed changes
@velzevur velzevur dismissed stale reviews from uwiger and hanssv via 69b1ebb March 24, 2023 12:57
uwiger
uwiger previously approved these changes Mar 24, 2023
View reviewed changes
rumyanar
rumyanar approved these changes Apr 4, 2023
View reviewed changes
Copy link
Contributor

@rumyanar rumyanar left a comment

Choose a reason for hiding this comment

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

Tested and it works ok with the new Master changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rumyanar rumyanar rumyanar approved these changes

@hanssv hanssv hanssv left review comments

@uwiger uwiger uwiger left review comments

@loxs loxs Awaiting requested review from loxs

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add ability to retrieve whole contract state of the GA contract, more improvement suggestions
5 participants
@velzevur @loxs @hanssv @uwiger @rumyanar