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

Verify Info Server refactoring and testing #2

Merged
merged 20 commits into from
Nov 28, 2018
Merged

Conversation

FrancisMurilloDigix
Copy link
Contributor

Main objective:

  • Refactor verify_info_server_request as a middleware or action filter
  • Add tests for info server and challenge requests for primarily user controller

Secondary objective:

  • Use rubocop for formatting and linting
  • Use factory_bot for fixtures
  • Use simple_cov for code coverage
  • Add proven column to challenges to prevent re-proving a proven challenge

assert_not user.update(address: address.slice(1)),
'should not fall below 42 characters'

assert_not user.update(address: 'g' + address.slice(2) + 'G'),
Copy link
Contributor

Choose a reason for hiding this comment

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

should be '0xg' + address.slice(4) + 'G' ?

validates :address,
presence: true,
uniqueness: true,
format: { with: /\A0x[a-fA-F0-9]{40}\Z/,
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, we should use the checksummed version of the address to store in the database. We can use the gem ethereum to convert and check checksummed addresses
Specifications: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-55.md

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can I use Eth::Utils.valid_address? instead since it is already installed?

Reference to Github

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup

@FrancisMurilloDigix
Copy link
Contributor Author

Pushed the changes but Github is not updating the comments?


case result
when :invalid_data, :database_error
render json: { errors: challenge_or_error },
Copy link
Contributor

Choose a reason for hiding this comment

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

error instead of errors

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, also created result_response and error_response to signify a standard success or failure response.

@mrenoon mrenoon merged commit 4abc5c8 into dev Nov 28, 2018
@mrenoon mrenoon deleted the feature/nonce-refactoring branch November 28, 2018 07:43
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.

None yet

2 participants