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

WIP: fix balanceOf #3147

Merged
merged 11 commits into from
May 16, 2019
Merged

WIP: fix balanceOf #3147

merged 11 commits into from
May 16, 2019

Conversation

nfurfaro
Copy link
Contributor

Description

This is just a start in the right direction. Still have some work to do to get tests passing again.

Issues

Fixes #3089

Checklist:

  • 1 PR, 1 purpose: my Pull Request applies to a single purpose
    • This PR only contains configuration changes (package.json, etc.)
    • This PR only contains code changes (if configuration changes are required, do a separate PR first, then re-base)
  • My code follows the style guidelines of this project, including naming conventions
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • If my code adds or changes components, I have written corresponding stories with Storybook
  • I have performed a self-review of my own code
  • If my code involves visual changes, I am adding applicable screenshots to this thread

@@ -225,6 +221,7 @@ contract('Lock / erc721 / transferFrom', accounts => {
from,
})
ID = await locks['FIRST'].getTokenIdFor.call(from)
console.log(ID)
Copy link
Contributor Author

@nfurfaro nfurfaro May 13, 2019

Choose a reason for hiding this comment

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

@HardlyDifficult ID at this point is 0, but should actually be a valid tokenID!
D0 we need to "wait" for mining before querying the contract perhaps?

@nfurfaro
Copy link
Contributor Author

@HardlyDifficult I'm stuck on this atm...here's the latest build log. https://circleci.com/gh/unlock-protocol/unlock/46868?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link As you can see, 2 failing tests. The second one is Uncaught Error: Returned values aren't valid, did it run Out of Gas? at ABICoder.decodeParameters (node_modules/truffle/build/webpack:/~/web3-eth-abi/src/index.js:226:1) which shows up in a thread about a web3 error : web3/web3.js#1916 (comment)

I need to take a break from this, it's killing me. If you know what's going on here off the top of your head, well that would be swell.

@HardlyDifficult
Copy link
Contributor

That second error, 'out of gas', is because of the first error. So that's the one to focus on. hmm I'm not sure which line is failing though - wish the stack trace didn't suck =p.

Maybe add some console logs to narrow it down

@nfurfaro
Copy link
Contributor Author

Yeah the stack trace does suck. Ive been playing with adding console.logs, as well as commenting out lines, to try to isolate the failure. Will keep at it after a break.

@HardlyDifficult
Copy link
Contributor

@nfurfaro fixed the gas report - sorry for the trouble. not exactly sure how it was working previously ;)

@nfurfaro
Copy link
Contributor Author

@HardlyDifficult Thanks! I wasn't sure what was causing that error, but happy to move on!

@nfurfaro nfurfaro requested a review from HardlyDifficult May 16, 2019 03:39
Copy link
Contributor

@HardlyDifficult HardlyDifficult left a comment

Choose a reason for hiding this comment

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

LG

@nfurfaro nfurfaro merged commit 1c87684 into master May 16, 2019
@nfurfaro nfurfaro deleted the n44o-fix-balanceOf branch May 16, 2019 14:39
nfurfaro added a commit that referenced this pull request May 16, 2019
nfurfaro added a commit that referenced this pull request May 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BalanceOf() returns 1 for previous key owner.
2 participants