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

only check nonces of transactions who's from address match the txMeta #2817

Merged
merged 3 commits into from
Dec 29, 2017

Conversation

frankiebee
Copy link
Contributor

fixes #2810

the issue seemed to be that we were checking against all confirmed transactions instead of just the ones for that specific address

@2-am-zzz
Copy link
Contributor

Is it possible for us to write tests for this? I'd like for us to get into the habit of writing tests for our PRs.

Copy link
Contributor

@2-am-zzz 2-am-zzz left a comment

Choose a reason for hiding this comment

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

Other than the small descript changes, this should be good to go.

}
})

it('should return false', function (done) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we clarify when these should return false?

.catch(done)
})

it('should return true', function (done) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same for here when they return true. Sorry for being pedantic, but future generations will thank us.

@frankiebee frankiebee merged commit b2e4f91 into master Dec 29, 2017
@2-am-zzz 2-am-zzz deleted the i#2810 branch January 5, 2018 23:23
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.

2 participants