arablocks / ara-contracts Goto Github PK

Expected Behavior

Method for getting the owner of job (in order to allow a peer to validate that another peer is the owner of a job)

Actual Behavior

Steps to Reproduce the Problem

Specifications

• Version:
• Platform:
• Subsystem:

Expected Behavior

Options -D, -s, -n, and -k should work globally

Actual Behavior

Using these options results in Unknown argument error due to use of strict() (result of #117)

Steps to Reproduce the Problem

1. Run any CLI command using one of those options

We should provide CLI documentation for this repo directly in the README for end users.

Expected Behavior

All contracts should use Ownable.sol found here.

Actual Behavior

Contracts use isolated ownership logic that doesn't have functionality for transferring or renouncing ownership.

Expected Behavior

when act library <did> fails, show error message

Actual Behavior

onfatal is not defined, is the error received, not why act library <did> failed

Steps to Reproduce the Problem

1. https://github.com/AraBlocks/ara-contracts/blob/master/bin/act-library#L60

    at Object.onall [as handler] (/Home/ara/ara-contracts/bin/act-library:60:5)```

## Specifications

  - Version: 0.2.7
  - Platform: macos
  - Subsystem: 10.12.6

web3 dependency update breaks everything

All references to ara-context should go through ara-util

(I think the only reference is here: https://github.com/AraBlocks/ara-contracts/blob/master/token.js#L4)

Specifications

• Version: 0.4.15

Expected Behavior

isValidArray function should use passed in fn to validate elements

Actual Behavior

isValidArray returns true if Array, but does not factor in fn results

Suggested fix

function isValidArray(arr, fn) {
  if (arr && Array.isArray(arr) && arr.length > 0) {
    let valid = true
    arr.forEach((args) => {
      valid = valid && fn(args)
    })
    return valid
  }
  return false
}

Also, in rewards.js in allocate, farmers needs to be let, since variable changes in isValidArray callback.

Expected Behavior

in rewards.js, getBalance and redeem should take in farmerDid

Actual Behavior

in rewards.js, getBalance and redeem takes in requesterDid

Expected Behavior

Incorrect combination of CL args should throw error

$ echo 'pass' | act token transfer did:ara:8a98c8305035dcbb1e8fa0826965200269e232e45ac572d26a45db9581986e67 $(aid whoami) 1000
 ara: error: 'transfer' is not a act token command. See 'act token--help'.
 ara: error: An error occured when invoking 'transfer' command. See 'act token --help'.

Actual Behavior

$ echo 'pass' | act token transfer did:ara:8a98c8305035dcbb1e8fa0826965200269e232e45ac572d26a45db9581986e67 $(aid whoami) 1000

Just hangs forever.

Steps to Reproduce the Problem

Specifications

• Version: 0.4.6
• Platform: macos
• Subsystem: bash version 4.4.19

Expected Behavior

Calls to ara-util.validate don't rely on constants

Actual Behavior

Calls rely on constants

Steps to Reproduce the Problem

1. n/a

Specifications

• Version: 0.2.4
• Platform: n/a
• Subsystem: n/a

Expected Behavior

submit in rewards.js should have info statements print values in units of Ara.

Actual Behavior

in the following statement, _budget is not in units of Ara.

_budget in

    await proxyContract.events.BudgetSubmitted({ fromBlock: 'latest', function(error) { debug(error) } })
      .on('data', (log) => {
        const { returnValues: { _did, _jobId, _budget } } = log
        info(requesterDid, 'budgeted', _budget, 'tokens for job', _jobId)
      })
      .on('changed', (log) => {
        debug(`Changed: ${log}`)
      })
      .on('error', (log) => {
        debug(`error:  ${log}`)
      })

Steps to Reproduce the Problem

Specifications

• Version:
• Platform:
• Subsystem:

Expected Behavior

Should be able to call purchase on an AFS contract with price set to null.

Actual Behavior

Throws an err
Error purchasing item: TypeError: Value must be greater than 0 at _validateApprovalOpts

Estimate options for these rewards functions are needed to warn user of cost before downloading or updating and afs

truffle migrate deploys all necessary contracts except the standard. This has to be done in a separate command. Including it in the migration after the rest of the contracts are deployed would make all of this possible in one step.

The proxyExists function internally calls getProxyAddress and returns a boolean, so everywhere we need to call getProxyAddress, we also need to call proxyExists before it, introducing overhead and a potential gotcha.

This issue proposes deprecating the proxyExists method and modifying the getProxyAddress function to just throw an Error if the requested proxy doesn't exist.

An estimate option would probably be good to have for users, that way we can give them an estimate cost for redeeming rewards instead of sending a tx immediately.

Expected Behavior

Purchase cli should succeed even without a job

Actual Behavior

Purchase cli fails if job is not true and prints:
ara: error: fatal: Expecting job Id.

(This is because purchase.js requires that job is an object, and purchase cli passes in an empty but valid object if job is false.

  } else if (opts.job && 'object' !== typeof opts.job) {
    throw TypeError('Expecting job object.')
  }

Steps to Reproduce the Problem

1. call act purchase with job not set

Specifications

• Version:
• Platform:
• Subsystem:

Expected Behavior

deployProxy method should be able to handle estimating cost without needing to have a valid did passed in.

Actual Behavior

Steps to Reproduce the Problem

Specifications

• Version:
• Platform:
• Subsystem:

Expected Behavior

allocate in rewards.js should pass strings to expandTokenValue, or expandTokenValue should accept non-strings.

Actual Behavior

expandTokenValue in token.js expects a string, but allocate in rewards.js passes in numbers.

  const validRewards = isValidArray(rewards, (reward) => {
    if (reward <= 0) {
      return false
    }
  })

  rewards = rewards.map(i => token.expandTokenValue(i))

A helper function that returns a boolean indicating is a proxy has not been committed to.

AFS standard should allow users to convert/recycle their rewards balance into a budget

If you pass price_ as a third parameter into the Purchased event, it will be much faster to calculate earnings. Currently to get earnings, I have to query all Purchased and PriceSet events from the contract. These are not trivial, and they each take a good amount of time. Then I have to loop through each Purchased event, look at the block number, and match it up with a PriceSet based on its block number, furthering time complexity.

By adding price to the emit, this process can be sped up alot, and will make for a faster app experience for users. It should only cost a few hundred more gas, which is really a very small fee I think.

For the dapp, there is a 'Published' view in the ui, that displays all of the files published under the AID of the user. Currently, there's no mapping of owners directly to their afs. This would be a really nice feature to have for the dapp, and probably for any other projects built off ara and the probably for the users themselves. We could just make a call to one of the contracts, and return an array of did we could render to the app.

Expected Behavior

-h / --help will always work without passing any options and exits successfully

Actual Behavior

If a command uses positionals, the command exits with code 1

Steps to Reproduce the Problem

1. Run any command with required options or positionals
2. Run echo $?

Specifications

• Version: 0.2.7

                       === npm audit security report ===                        
                                                                                
┌──────────────────────────────────────────────────────────────────────────────┐
│                                Manual Review                                 │
│            Some vulnerabilities require your attention to resolve            │
│                                                                              │
│         Visit https://go.npm.me/audit-guide for additional guidance          │
└──────────────────────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate      │ Sandbox Breakout / Arbitrary Code Execution                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ static-eval                                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ No patch available                                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ 175f97706277aaebd67361d0a54f282e5ef2f8f37e275d6e48cede031d8… │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ 175f97706277aaebd67361d0a54f282e5ef2f8f37e275d6e48cede031d8… │
│               │ > brfs > static-module > static-eval                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/758                       │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate      │ Sandbox Breakout / Arbitrary Code Execution                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ static-eval                                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ No patch available                                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ 5080b18143afb317a971b6e5f6094a1f69fd29530443f3af0ae7ff2013b… │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ 5080b18143afb317a971b6e5f6094a1f69fd29530443f3af0ae7ff2013b… │
│               │ >                                                            │
│               │ 175f97706277aaebd67361d0a54f282e5ef2f8f37e275d6e48cede031d8… │
│               │ > brfs > static-module > static-eval                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/758                       │
└───────────────┴──────────────────────────────────────────────────────────────┘

Specifications

• Version: 0.23.0
• Platform:
• Subsystem:

Contract migration should update contract addresses accordingly depending on the network deployed to (local, privatenet, testnet, mainnet).

Expected Behavior

truffle migrate --network testnet should automatically update constants.js with the new Testnet contract addresses.

Actual Behavior

Only privatenet addresses are updated after migration, regardless of selected network

Steps to Reproduce the Problem

1. Use privatenet provider
2. truffle migrate --network testnet
3. constants.js privatenet addresses are updated

If we index event parameters, we can use the filter option provided by web3 to query for events whose return values we specify. It's a minimal change and shouldn't cost much gas

event Transfer(address from, address to, uint256 value);

becomes

event Transfer(address indexed from, address indexed to, uint256 value);

If we don't index, querying becomes unscalable. This is the current function that sums all rewards a given user has earned for a given AFS.

const totalRewards = (await AFSContract.getPastEvents('Redeemed'))
  .reduce((sum, { returnValues }) =>
     returnValues._sender === userEthAddress
	  ? sum += Number(araContracts.token.constrainTokenValue(returnValues._amount))
	  : sum
 0)

It pulls back alllll the Redeemed events the contract ever fired then filters out the ones we don't want. Instead, we could do:

const totalRewards = (await AFSContract.getPastEvents('Redeemed', { filter: { _farmer: did }))
  .reduce((sum, { returnValues }) =>
    ? sum += Number(araContracts.token.constrainTokenValue(returnValues._amount))
    : sum
 0)

This only pulls back the Redeemed events for the given DID.

https://web3js.readthedocs.io/en/1.0/web3-eth-contract.html#getpastevents
https://www.reddit.com/r/ethdev/comments/78ugki/do_indexed_parameters_in_events_cost_more_gas/

Expected Behavior

allocate should take in farmer DIDs and resolve the addresses

Actual Behavior

allocate takes in addresses

Expected Behavior

When a user deploys a proxy, it should be added to their library.

Actual Behavior

Steps to Reproduce the Problem

Specifications

• Version:
• Platform:
• Subsystem:

Feature Request

Add method in AFS contract to check if user has purchased an AFS. This would be helpful for a quick (free) way to validate if a user has the right to download a piece of content. The only alternatives currently are iterating through a users library, or iterating through purchase events, which can be time consuming.

Expected Behavior

In order to use the jobId that is generated during purchase, the purchase method should return the jobId.

Expected Behavior

When calling submit or allocate, should error if user has not purchased content

Actual Behavior

Submit and Allocate revert without reason if have not purchased content

Steps to Reproduce the Problem

Specifications

• Version:
• Platform:
• Subsystem:

Expected Behavior

$ act purchase <purchaser> <did>
...
ara: error: fatal: ongoing transaction; failed to submit the new transaction

Actual Behavior

$ act purchase <purchaser> <did>
...
ara: error: fatal: replacement transaction underpriced

Steps to Reproduce the Problem

1. Make two purchases at the same time via CLI

Specifications

• Version: 0.11.3
• Platform:
• Subsystem: