Remove contract addresses from Database and add to Node.
Rename the config::load module to parse.
Update tests and README.
Side Comments and Suggestions
The Error type needs to be redesigned using the failure crate rather than error_chain. error_chain encourages the use of string errors, which are largely useless to library consumers. Precise types are needed for every error. String-only errors should be forbidden.
Formatting needs improvement (tabs -> spaces, line width).
Fields within Database, Config, and many other structs are declared pub. Leaving struct fields public is rarely a good idea, it's convenient at first but it can create situations where fields could possibly be accidentally mutated when they shouldn't be. This might not seem to be possible for many of the structs because they're generally contained in an App and therefore an Arc, making them immutable in that case, but the idiomatic solution is to create getter methods (and setter when necessary). Who knows what future library changes could accidentally expose a field to mutation in an unintended way. If fields do not need to be exposed publicly pub(crate) is an ok solution but can still leaves the door open for potential future bugs to creep in.
The tests within integration-tests fail. This has nothing to do with the changes made here. I don't see an issue for this.
If reviewers feel the above suggestions warrant more attention, let me know and I will create issues for each (and happily tackle those issues).
Changes
Database
and add toNode
.config::load
module toparse
.Side Comments and Suggestions
Error
type needs to be redesigned using thefailure
crate rather thanerror_chain
.error_chain
encourages the use of string errors, which are largely useless to library consumers. Precise types are needed for every error. String-only errors should be forbidden.Database
,Config
, and many other structs are declaredpub
. Leaving struct fields public is rarely a good idea, it's convenient at first but it can create situations where fields could possibly be accidentally mutated when they shouldn't be. This might not seem to be possible for many of the structs because they're generally contained in anApp
and therefore anArc
, making them immutable in that case, but the idiomatic solution is to create getter methods (and setter when necessary). Who knows what future library changes could accidentally expose a field to mutation in an unintended way. If fields do not need to be exposed publiclypub(crate)
is an ok solution but can still leaves the door open for potential future bugs to creep in.integration-tests
fail. This has nothing to do with the changes made here. I don't see an issue for this.If reviewers feel the above suggestions warrant more attention, let me know and I will create issues for each (and happily tackle those issues).
Closes #101