AIP-011: Ownable Smart Contract

[patitonar]

Title: Ownable Smart Contract

Author(s): Manuel Garcia, Pavel Orda

Type: ASC (Aion Standards and Conventions)

Status: REVIEW

Creation Date: May 28th, 2019

Contact Information: [email protected]

Summary

Contract module for simple authorization and access control mechanisms.

Value Proposition

Contract module which provides a basic access control mechanism, where there is an account (an owner) that can be granted exclusive access to specific functions.

Motivation

This module could be used through inheritance. It will make available the modifier onlyOwner, which can be aplied to your functions to restrict their use to the owner.

Non-Goals

Success Metrics

Description

Specification

Based on Solidity implementation https://github.com/OpenZeppelin/openzeppelin-solidity/blob/master/contracts/ownership/Ownable.sol

Logic

Risks & Assumptions

Test Cases

Test cases reference implementation

Implementations

Definitions

Reference implementation

Methods

getOwner function

Returns the address of the current owner.

returns: Address of the owner

public static Address getOwner()

renounceOwnership function

Leaves the contract without owner. Can only be called by the current owner.

Note: Renouncing ownership will leave the contract without an owner, thereby removing any functionality that is only available to the owner.

public static void renounceOwnership()

transferOwnership function

Transfers ownership of the contract to a new account (newOwner). Can only be called by the current owner.

public static void transferOwnership(Address newOwner)

parameters
tokenHolder: Address for which the balance is returned.

onlyOwner function

Stops transaction if called by any account other than the owner. Do not work for function calls

private static void onlyOwner()

OwnershipTransferred event

Indicates ownership change form oldOwner address to newOwner address.

parameters
oldOwner: Address of previous owner.
newOwner: Address of new owner.

private static void emitOwnershipTransferredEvent(Address oldOwner, Address newOwner)

Dependencies

Copyright

All AIP’s are public domain. Copyright waiver to be linked to https://creativecommons.org/publicdomain/zero/1.0/

[jennijuju]

For the constants NO_ADDRESS and OWNERSHIP_TRANSFERRED below,

 public static class Const {
        public static final Address NO_ADDRESS = new Address(new byte[32]);
        public static final String OWNERSHIP_TRANSFERRED = "OwnershipTransferred";
    }
    private static Address owner = NO_ADDRESS;

Will it be better to just assign them as constants instead of putting them into a static class, since extra class namespace will cost more energy? Also, it will be cheaper to have String OWNERSHIP_TRANSFERRED to be defined as a byte[]. Like:

     private static final Address NO_ADDRESS = new Address(new byte[32]);
     private static final byte[] OWNERSHIP_TRANSFERRED = "OwnershipTransferred".getBytes();

[jennijuju]

 private static boolean isOwner() {
        Address caller = Blockchain.getCaller();
        return owner.equals(caller);
    }

I think isOwner() should be a public @Callable function to allow people to check if they are the owner of the contract. It can also be cheaper if we don't assign a new variable as Blockchain.caller() .

@Callable
 public static boolean isOwner() {
        return owner.equals( Blockchain.getCaller();
    }

[jennijuju]

 private static void onlyOwner() {
        Blockchain.require(isOwner());
 }

  private static boolean isOwner() {
        Address caller = Blockchain.getCaller();
        return owner.equals(caller);
 }

It maybe cheaper if just do

private static void onlyOwner() }
   Blockchain.require(Blockchain.getcaller().equals(owner));
}

[jennijuju]

Hey @patitonar, thank you for submitting the AIP. This AIP now is ready to be reviewed by Aion community.

[xiwix]

I think isOwner() should be a public @Callable function to allow people to check if they are the owner of the contract.

@Callable
public static boolean isOwner() {
    return owner.equals( Blockchain.getCaller();
}

Blockchain.getCaller() is not returning real address in non-transactions and transactions are not returning response. So there is no need to make isOwner() @Callable.

It can also be cheaper if we don't assign a new variable as Blockchain.caller() .

Java compiler will do this automatically.

[xiwix]

For the constants NO_ADDRESS and OWNERSHIP_TRANSFERRED below,

 public static class Const {
        public static final Address NO_ADDRESS = new Address(new byte[32]);
        public static final String OWNERSHIP_TRANSFERRED = "OwnershipTransferred";
    }
    private static Address owner = NO_ADDRESS;

Will it be better to just assign them as constants instead of putting them into a static class, since extra class namespace will cost more energy? Also, it will be cheaper to have String OWNERSHIP_TRANSFERRED to be defined as a byte[]. Like:

     private static final Address NO_ADDRESS = new Address(new byte[32]);
     private static final byte[] OWNERSHIP_TRANSFERRED = "OwnershipTransferred".getBytes();

I removed Const class. But kept String for easier testing.

[patitonar]

@jennijuju Thanks for the feedback! This PR https://github.com/protofire/AIP-011-Ownable/pull/4/files add the suggested changes