-
Notifications
You must be signed in to change notification settings - Fork 0
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
Initial work on KMaaS #1
Conversation
3cc94da
to
fc416de
Compare
fc416de
to
e13afc7
Compare
If you're running on an Apple M chip, you can use.
If that doesn't work let me know what the problem was and we can get it working for you. |
contracts/Account.sol
Outdated
_; | ||
} | ||
|
||
function grantPermission(address grantee, uint256 expiry) public virtual override authorized { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I temporarily grant permission to somebody, they can use updateController
to permanently take control over the contract, rather than having temporary access (as indicated by the expiry
). ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. I think only the controller should be able to call updateController.
contracts/Account.sol
Outdated
function clone (address starterOwner) | ||
public virtual override | ||
returns (AccountBase acct) { | ||
AccountBase acc = new Account(starterOwner); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's best to use a lightweight clone factory
See: https://eips.ethereum.org/EIPS/eip-1167
This will reduce the cost of clone
significantly as the code for Account
will only be deployed once, then every time clone
is called a proxy contract will be initialized which does DELEGATECALL
into the Account
contract. This does mean that initialization needs to be moved out of the constructor etc.
May I suggest as interesting reading: |
contracts/Account.sol
Outdated
_; | ||
} | ||
|
||
function grantPermission(address grantee, uint256 expiry) public virtual override authorized { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. I think only the controller should be able to call updateController.
// Delete a named symmetric key. | ||
function deleteSymKey(string calldata name) | ||
external virtual authorized { | ||
keys[name] = Key.wrap(bytes32(0)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a matter of style preferance, but I think delete keys[name]
is cleaner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think because Key is a user defined type the delete operation isn't defined for it
contracts/Sender.sol
Outdated
// SPDX-License-Identifier: MIT | ||
pragma solidity ^0.8.9; | ||
|
||
contract Sender { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this for testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it is
I tried that command when trying to set up a local env and the problems were:
|
Can you provide a console log which includes the command you're running to launch the Docker and the version of OSX + chip/model you're using, so I can file a ticket for this and investigate? |
Weird, we're investigating, I've never seen Postgres fail to start, the latest version of If you can get a shell inside the localnet container you can |
Alternatively if you have lots of dockers already running, maybe Docker Desktop is running out of memory. |
Yep - I didn't have other containers running but after pulling the latest image and running |
Ok. So please run with |
8fcb214
to
ebe4e56
Compare
ebe4e56
to
c81a14e
Compare
This PR is largely based on the KMaaS one-pager and API spec. So far the basic functionality in terms of the
Account
,AccountWithSymKey
, andValidator
seems to work. I'm still working on adding more tests and want to do so before this PR is merged.I wasn't able to get sapphire-localnet working on my local setup so the way that I've been testing is having two different accounts for Sapphire testnet, funding both, and setting
PRIVATE_KEY_1
andPRIVATE_KEY_2
to each of their respective private keys.