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

CU-86dt24n2g - Allow user to add/change icon link if they can verify … #10

Merged
merged 1 commit into from
Apr 8, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
CU-86dt24n2g - Allow user to add/change icon link if they can verify …
…themselves on the smart contract
  • Loading branch information
luc10921 committed Apr 2, 2024
commit d666c787ca39522645a4fe295285210f1592fd0b
20 changes: 18 additions & 2 deletions contract/icondapp.manifest.json
Original file line number Diff line number Diff line change
@@ -5,7 +5,7 @@
"methods": [
{
"name": "_deploy",
"offset": 2014,
"offset": 2012,
"parameters": [
{
"type": "Any",
@@ -198,9 +198,25 @@
"safe": false,
"returntype": "Boolean"
},
{
"name": "canChangeMetaData",
"offset": 1582,
"parameters": [
{
"type": "Hash160",
"name": "contract_script_hash"
},
{
"type": "Hash160",
"name": "contract_owner"
}
],
"safe": true,
"returntype": "Boolean"
},
{
"name": "listIcons",
"offset": 1951,
"offset": 1949,
"parameters": [],
"safe": true,
"returntype": "InteropInterface",
Binary file modified contract/icondapp.nef
Binary file not shown.
61 changes: 32 additions & 29 deletions contract/icondapp.py
Original file line number Diff line number Diff line change
@@ -4,6 +4,7 @@
from boa3.builtin.interop import runtime, storage
from boa3.builtin.interop.blockchain import Transaction
from boa3.builtin.interop.contract import Contract
from boa3.builtin.interop.contract.contractmanifest import ContractAbi
from boa3.builtin.interop.contract.contractmanifest import ContractManifest
from boa3.builtin.interop.iterator import Iterator
from boa3.builtin.nativecontract.contractmanagement import ContractManagement
@@ -248,44 +249,46 @@ def get_contract_owner(script_hash: UInt160) -> Optional[UInt160]:

@public(name='setOwnership')
def set_ownership(script_hash: UInt160, contract_owner: UInt160) -> bool:
# if it's not icon dapp admin, needs to check if it's the contract deployer
if runtime.check_witness(get_owner()):
contract_deployer = contract_owner
else:
current_contract_owner: UInt160 = get_contract_owner(script_hash)
# using two if's instead of one with a 'and' operation because it had runtime errors
if isinstance(current_contract_owner, UInt160):
# the contract owner can change the contract ownership
# if it's not signed by the contract owner, checks if it's signed by the given address
if not runtime.check_witness(current_contract_owner):
if not runtime.check_witness(contract_owner):
raise Exception('No authorization')

contract_deployer = contract_owner
else:
if not runtime.check_witness(contract_owner):
raise Exception('No authorization')

contract_deployer = get_deployer(script_hash, contract_owner)

if isinstance(contract_deployer, UInt160):
if can_change_meta_data(script_hash, contract_owner):
contract_owner_key = get_contract_owner_key(script_hash)
storage.put(contract_owner_key, contract_deployer)
storage.put(contract_owner_key, contract_owner)
return True
return False


def get_deployer(script_hash: UInt160, sender: UInt160) -> Optional[UInt160]:
contract: Contract = ContractManagement.get_contract(script_hash)
@public(name='canChangeMetaData', safe=True)
def can_change_meta_data(contract_script_hash: UInt160, contract_owner: UInt160) -> bool:
Copy link

Choose a reason for hiding this comment

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

Consider renaming the function can_change_meta_data to can_change_metadata to follow the camelCase naming convention.

Copy link
Member

@melanke melanke Apr 8, 2024

Choose a reason for hiding this comment

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

Metadata is a single word. We should change on the annotation as well to canChangeMetadata

contract: Contract = ContractManagement.get_contract(contract_script_hash)
if not isinstance(contract, Contract):
return None
return False

icon_dapp_owner = get_owner()
melanke marked this conversation as resolved.
Show resolved Hide resolved
if runtime.check_witness(icon_dapp_owner):
return True

computed_script_hash: bytes = _compute_contract_hash(sender, contract)
if computed_script_hash != script_hash:
return None
contract_abi: ContractAbi = contract.manifest.abi
has_verify = False
for method in contract_abi.methods:
melanke marked this conversation as resolved.
Show resolved Hide resolved
if method.name == 'verify':
has_verify = True
break
if has_verify:
if runtime.check_witness(contract_script_hash):
Copy link

Choose a reason for hiding this comment

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

Consider handling the case where runtime.check_witness(contract_script_hash) returns None. This could potentially lead to a NoneType error.

Copy link
Member

@melanke melanke Apr 8, 2024

Choose a reason for hiding this comment

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

Is it possible for the check_witness to return None?

return True
else:
return False

current_contract_owner: UInt160 = get_contract_owner(contract_script_hash)
if isinstance(current_contract_owner, UInt160):
if runtime.check_witness(current_contract_owner):
Copy link

Choose a reason for hiding this comment

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

Consider handling the case where runtime.check_witness(current_contract_owner) returns None. This could potentially lead to a NoneType error.

Copy link
Member

@melanke melanke Apr 8, 2024

Choose a reason for hiding this comment

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

Again, is it possible for the check_witness to return None?

return True

return sender
# check if the contract was deployed by the given address
computed_script_hash: bytes = _compute_contract_hash(contract_owner, contract)
Copy link

Choose a reason for hiding this comment

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

Consider handling the case where _compute_contract_hash(contract_owner, contract) returns None. This could potentially lead to a NoneType error when comparing computed_script_hash with contract_script_hash.

Copy link
Member

@melanke melanke Apr 8, 2024

Choose a reason for hiding this comment

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

Is it possible for the _compute_contract_hash to return None?

if computed_script_hash == contract_script_hash:
return True

return False

def _compute_contract_hash(sender: UInt160, contract: Contract) -> bytes:
# there's a bug with calling contract.nef[:4] directly
17 changes: 17 additions & 0 deletions default.neo-express
Original file line number Diff line number Diff line change
@@ -71,6 +71,23 @@
}
}
]
},
{
"name": "otherUser",
"accounts": [
{
"private-key": "d9d688984751075b49741a2add7d4a195bc942ee32209328eceed8bc91931a61",
"script-hash": "NPR1tgkQcG8GxXkLrTAJKcuq5anro97eu7",
"label": null,
melanke marked this conversation as resolved.
Show resolved Hide resolved
"is-default": true,
melanke marked this conversation as resolved.
Show resolved Hide resolved
"contract": {
"script": "0c2103bcbf1953c65788c303035df44d340dbd804cabeafe2c76577ff6f35bc4121de64156e7b327",
"parameters": [
"Signature"
]
}
}
]
}
],
"settings": {}
Binary file modified postSetup.neoxp-checkpoint
Binary file not shown.
18 changes: 9 additions & 9 deletions sdk/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions sdk/package.json
Original file line number Diff line number Diff line change
@@ -11,7 +11,9 @@
"publishConfig": {
"access": "public"
},
"files": ["dist"],
"files": [
"dist"
],
"scripts": {
"build": "tsc",
"test": "ts-mocha tests/**/*.spec.ts",
@@ -26,7 +28,7 @@
"license": "MIT",
"dependencies": {
"@cityofzion/neon-core": "5.5.1",
"@cityofzion/neon-dappkit": "0.3.1",
"@cityofzion/neon-dappkit": "0.4.1",
"@cityofzion/neon-dappkit-types": "0.3.1",
"@types/lodash": "^4.14.176",
"@types/node": "^16.11.11",
23 changes: 18 additions & 5 deletions sdk/src/IconDapp.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Neo3EventListener, Neo3Invoker, Neo3Parser, TypeChecker } from "@cityofzion/neon-dappkit-types"
import { Neo3EventListener, Neo3Invoker, Neo3Parser, Signer, TypeChecker } from "@cityofzion/neon-dappkit-types"
import * as Invocation from './api'
import { ScriptHashAndIcons, IconProperties } from './types'

@@ -254,20 +254,33 @@ export class IconDapp{
/**
* Sets the owner of a smart contract. If sender is not the owner of the smart contract, then it will return false.
*/
async setOwnership(params: { scriptHash: string, contractOwner: string } ): Promise<string>{
async setOwnership(params: { scriptHash: string, contractOwner: string }, signers: Signer[] = [] ): Promise<string>{
return await this.config.invoker.invokeFunction({
melanke marked this conversation as resolved.
Show resolved Hide resolved
invocations: [Invocation.setOwnershipAPI(this.config.scriptHash, params, this.config.parser)],
signers: [],
signers,
})
}

/**
* Sets the owner of a smart contract. If sender is not the owner of the smart contract, then it will return false.
*/
async testSetOwnership(params: { scriptHash: string, contractOwner: string } ): Promise<boolean>{
async testSetOwnership(params: { scriptHash: string, contractOwner: string }, signers: Signer[] = [] ): Promise<boolean>{
const res = await this.config.invoker.testInvoke({
melanke marked this conversation as resolved.
Show resolved Hide resolved
invocations: [Invocation.setOwnershipAPI(this.config.scriptHash, params, this.config.parser)],
signers: [],
signers,
})

if (res.stack.length === 0) {
throw new Error(res.exception ?? 'unrecognized response')
melanke marked this conversation as resolved.
Show resolved Hide resolved
}

return this.config.parser.parseRpcResponse(res.stack[0], { type: 'Boolean' })
}

async canChangeMetaData(params: { contractScriptHash: string, contractOwner: string }, signers: Signer[] = []): Promise<boolean>{
const res = await this.config.invoker.testInvoke({
melanke marked this conversation as resolved.
Show resolved Hide resolved
invocations: [Invocation.canChangeMetaDataAPI(this.config.scriptHash, params, this.config.parser)],
signers,
})

if (res.stack.length === 0) {
9 changes: 9 additions & 0 deletions sdk/src/api.ts
Original file line number Diff line number Diff line change
@@ -128,6 +128,15 @@ export function setOwnershipAPI(scriptHash: string, params: { scriptHash: string
}
}

export function canChangeMetaDataAPI(scriptHash: string, params: { contractScriptHash: string, contractOwner: string }, parser: Neo3Parser ): ContractInvocation {
return {
scriptHash,
operation: 'canChangeMetaData',
args: [parser.formatRpcArgument(params.contractScriptHash, { type: 'Hash160' }),parser.formatRpcArgument(params.contractOwner, { type: 'Hash160' }),
],
}
}

export function listIconsAPI(scriptHash: string): ContractInvocation {
return {
scriptHash,
Loading
Loading