Conversation
Ivshti
left a comment
There was a problem hiding this comment.
That's a great job, amazing!
But there's a few things that need to be addressed
test/integration.js
Outdated
|
|
||
| tape('submit events and ensure they are accounted for', async function(t) { | ||
| const evs = genEvents(3).concat(genEvents(2, 'anotherPublisher')) | ||
| const evs = genEvents(3).concat(genEvents(2, '0x48b6d5748885988cb657a06647c05fc2af1ffacf')) |
There was a problem hiding this comment.
maybe put this in the dummy data constants (dummyVals)
test/integration.js
Outdated
| await testRejectState(t, 'InvalidSignature', function(newState) { | ||
| // increase the balance, so we effectively end up with a new state | ||
| const balances = { ...newState.balances, someoneElse: '1' } | ||
| const balances = { ...newState.balances, '0x033ed90e0fec3f3ea1c9b005c724d704501e0196': '1' } |
There was a problem hiding this comment.
again, consider putting this in the dummyVals but I have an important question here:
how do we make sure that 0x033 does not override something that's already in newState.balances?
also, in the second case, how do we ensure that '0x033' was not previously present in the newState.balances
how about generating a random addr in the second case?
There was a problem hiding this comment.
I see now that this is the creator from dummyVals
I think that you should indeed gen a random addr here, in this test and in the next (const fakeBalances=)
There was a problem hiding this comment.
Yeah true, I have added random address generation
test/lib.js
Outdated
| return Promise.all([exec(leaderTick), exec(followerTick)]) | ||
| } | ||
|
|
||
| function nonce() { |
There was a problem hiding this comment.
is there a more elegant way to do this?
There was a problem hiding this comment.
I made the change to use current timestamp as nonce
| withdrawPeriodStart: new Date('2200-01-01').getTime(), | ||
| minPerImpression: '1', | ||
| maxPerImpression: '1', | ||
| validators: [ |
There was a problem hiding this comment.
This seems to be removed because it was already in the dummyVals.channel.spec, correct?
194365e to
e7f364e
Compare
| `./bin/validatorWorker.js --single-tick --adapter=dummy --dummyIdentity=awesomeFollower --sentryUrl=http://localhost:8006` | ||
| ) | ||
| ]) | ||
| let leaderTick = `./bin/validatorWorker.js --single-tick --adapter=dummy --dummyIdentity=0xce07cbb7e054514d590a0262c93070d838bfba2e --sentryUrl=http://localhost:8005` |
There was a problem hiding this comment.
can we avoid hardcoding those here..we have dummyVals
| if (process.env.RUST_VALIDATOR_WORKER) { | ||
| leaderTick = `RUST_BACKTRACE=1 ${ | ||
| process.env.RUST_VALIDATOR_WORKER | ||
| } -a dummy -i 0xce07cbb7e054514d590a0262c93070d838bfba2e -u http://localhost:8005 -t` |
There was a problem hiding this comment.
can we avoid hardcoding those here..we have dummyVals
Fix #223