search

polymorpher / one-wallet

1wallet | Modulo OTP Wallet - unconventional keyless, non-custodial wallet secured by Google Authenticator. EVM-compatible, smart contract operated, with composable security.
https://1wallet.crazy.one
Apache License 2.0
112 stars 48 forks source link

More tests #207

Closed polymorpher closed 2 years ago

polymorpher commented 2 years ago

Currently, many functionalities supported by the contract are untested (e.g. call, multi-call, buy domain, command, etc.). I had to manually test them end-to-end using the full web client. This makes the test coverage rather limited, and the process is highly inefficient. More tests should be written so each category of functionalities are covered.

Exact list will be provided in the follow up comments.

johnwhitton commented 2 years ago

Development Tasks

Feedback

Development Issues/Tasks

// ==== Validation Helpers ==== const validateEvent = async ({ tx, expectedEvent, expectedData }) => { const events = ONEParser.parseTxLog(tx?.receipt?.rawLogs) const event = events.filter(e => e.eventName === expectedEvent)[0] const eventName = event?.eventName const data = event?.data if (expectedEvent) { assert.deepStrictEqual(expectedEvent, eventName, 'Expected event not triggered') } if (expectedData) { assert.deepStrictEqual(expectedData, data, 'Expected event data is different') } }


### Completed Tasks log
- [x] `SE-NEGATIVE-7-DISPLACE` fails on FORWARD operation when using a duration of 24 [log is here](https://gist.github.com/johnwhitton/374fc24bc89b9398ebbbdc25acd9fc23)
- [x] `SE-COMPLEX-7-0` and `InnerCores_New:` are failing when authenticating the otp after displacement
- [x] Review DISPLACE `testTime` logic overview to understand innerTrees

// testForTime Logic overview // Logic Overview example using 24 intervals of 30 seconds = 12 minutes duration // testTime: rounded to the nearest 30 seconds e.g. 05:02:30 // effectiveTime: testTime - half duration e.g. 04:56:30 (i.e. testTime - 6 minutes) // makeWallet: creates a wallet using the seed storing the root of 6 merkle trees (each with 4 passwords) in innerCores // makeCores: is called by makeWallet and populates the 24 otps into layers and innerTrees held in the client object // * makeCores: is called again with a newSeed to generate another 24 otps // for each of the (6) Trees (holding 4 passwords) // we increase t0 by 1 (needed to DISPLACE the cores) // we generate a new Root (needed to DISPLACE the cores) // we populate the data with the new Core Information (used in the Displace) // we generate the tOTP (time based one time password) from the original password set by using alice.seed // we calculate innerEffectiveTime and innerExpiryTime and reconstruct the otps array from the original otpb // otpb (is an array of the 24 one time passwords) // otpb: {"0":0,"1":6,"2":11,"3":217,"4":0,"5":13,"6":245,"7":73,"8":0,"9":0,"10":73,"11":83,"12":0,"13":13,"14":85,"15":212,"16":0,"17":2,"18":104,"19":150,"20":0,"21":11,"22":7,"23":74} // otps (is an array of 6 objects each holding four passords) // otps: [{"0":0,"1":6,"2":11,"3":217},{"0":0,"1":13,"2":245,"3":73},{"0":0,"1":0,"2":73,"3":83},{"0":0,"1":13,"2":85,"3":212},{"0":0,"1":2,"2":104,"3":150},{"0":0,"1":11,"2":7,"3":74}] // we calculate the index for the otp (based on the testTime, effectiveTime and duration) // we call displace which updates the wallet with 6 new Root entries in innercores, updates the Info, and OldInfo // we validate that the wallet was succesfully updated // endfor // we return the wallet(alice), newEffectiveTime and state (i.e. alices wallet with 24 new cores added and 6 oldInfos)

- [x] SE-NEGATIVE-7 sporadic reverts
 Error: Returned error: VM Exception while processing transaction: revert Proof is incorrect
  at commitReveal (test/util.js:979:46)
  at runMicrotasks (<anonymous>)
  at processTicksAndRejections (internal/process/task_queues.js:95:5)
  at Object.executeUpgradeTransaction (test/util.js:936:64)
  at Context.<anonymous> (test/security.js:385:23)

- [x] Truffle embeds chai for assertions
  - [x] We should also include reversion tests [example](https://github.com/hiddenstate/gama-contract/blob/master/test/GAMAWhitelistMint.js#L56) uses [chai-as-promised](https://www.chaijs.com/plugins/chai-as-promised/) 
- [x] `client.js` is failing see [this gist](https://gist.github.com/johnwhitton/b535fc4ce52f2ddf48af2cd7e5de62bf)
- [x] core `computeRecoveryHash` failing in `CO-BASIC-6 RECOVER-NEW:`
- [x] security `testTime2` failing
- [x] check on util `deploy(initArgs)` fails when saving wallets before taking a snapshot also may be related to  `innerCore.js` when running complete file.
- [x] `innerCores.js` is failing see [this gist](https://gist.github.com/johnwhitton/b535fc4ce52f2ddf48af2cd7e5de62bf#file-innercores-js)
- [x] REVOKE need to test revoking a specific signature, was unable to pass any valued which did not trigger revocation of all signatures.
- [x] RPC issues when running tokens.js `Could not connect to your Ethereum client with the following parameters:`
- [x] Occasional timestamp undefined issues `TypeError: Cannot read property 'timestamp' of null` **runing ganache with `-b 2` 2 second blocktime resolves this**
- [x] `getONEWalletState` objects creation need review (including spendingState, allCommits, trackedTokens, signatures)
- [x] updateOldSpendingState" needs review and refactoring
- [x] `updateOldTrackedTokens` needs review and refactoring
- [x] `WA.BASIC.8 FORWARD.expectedSpendingState` enhance with actual values to compare
- [x] `WA.COMPLEX.8.0 ` improve signature validation
- [x] WA.COMPLEX.8.0 ` check why carol's signatures aren't updated
- [x] Wallet tests get error deploying wallet when testing whole file `Error: Wallet deploy unsuccessful` **fixed if running ganache with `-b 2` blocktime of 2 seconds rather than automining**
- [x] command.js check whether the invalid signature validation is needed.
- [x] Validating multiple events e.g. SpendingLimitChanged and HighestSpendingLimitChanged
- [x]  JUMP_SPENDING_LIMIT fails `Error: Returned error: VM Exception while processing transaction: revert Proof is incorrect` (CHANGE_SPENDING_LIMIT works). it appears to be based on `function authenticate in reveal.sol` which says `            // if innerCores are empty, this operation (in this case) is doomed to fail. This is intended. Client should warn the user not to lower the limit too much if the wallet has no innerCores (use Extend to set first innerCores). Client should also advise the user the use Recovery feature to get their assets out, if they are stuck with very low limit and do not want to wait to double them each spendInterval.`
- [x] RECOVER needs additional authentication `  /// To initiate recovery, client should submit leaf_{-1} as eotp, where leaf_{-1} is the last leaf in OTP Merkle Tree. Note that leaf_0 = hasher(hseed . nonce . OTP . randomness) where hasher is either sha256 or argon2, depending on client's security parameters. The definition of leaf_{-1} ensures attackers cannot use widespread miners to brute-force for seed or hseed, even if keccak256(leaf_{i}) for any i is known. It has been considered that leaf_0 should be used instead of leaf_{-1}, because leaf_0 is extremely unlikely to be used for any wallet operation. It is only used if the user performs any operation within the first 60 seconds of seed generation (when QR code is displayed). Regardless of which leaf is used to trigger recovery, this mechanism ensures hseed remains secret at the client. Even when the leaf becomes public (on blockchain), it is no longer useful because the wallet would already be deprecated (all assets transferred out). It can be used to repeatedly trigger recovery on this deprecated wallet, but that would cause no harm.`
- [x]  WA.BASIC.9 RECOVER_SELECTED_TOKENS getting `revert Param mismatch` may need a new function in oneWallet to support dest and data `paramsHash = keccak256(bytes.concat(bytes32(bytes20(address(op.dest))), op.data));` **created computeDestDataHash**
polymorpher commented 2 years ago

Let's close this and open new issues for additional tests