Skip to content
This repository has been archived by the owner on Sep 18, 2020. It is now read-only.

refactor&more (feel free to review, still improving docs and tests) #159

Merged
merged 229 commits into from Aug 24, 2018
Merged

refactor&more (feel free to review, still improving docs and tests) #159

merged 229 commits into from Aug 24, 2018

Conversation

snd
Copy link
Contributor

@snd snd commented Apr 11, 2018 •
edited

  • moved logic common to the 3 logical relay processes (main to side sign,
    side to main sign, side to main submit signatures) into RelayStream
    • RelayStream for any process that listens to a LogStream
      and executes transactions for each log
  • changed naming of deposit withdraw to be more neutral (for message passing)
    • rust only
  • resolved merge home/foreign authority addresses #154
  • moved contracts into separate subcrate
  • less future boilerplate
    • see helpers::AsyncCall and related changes to ethabi
  • use https://github.com/paritytech/rust_solc to compile contracts
  • use RPC instead of IPC so parity nodes and bridge nodes can exist on separate machines
  • change naming from home/foreign to main/side as the previous naming
    tended to confuse most people
  • bridge nodes now check whether they have provided signatures before doing so
    • doesn't crash when catching up from an old state file
  • adapted to upstream changes to various crates
  • couple things less to Config and Database
  • refactor database abstractions
  • MainContract and SideContract highlevel interfaces that are used by relay streams instead of scattering that logic throughout the relay streams
  • split relay streams into more focused individual relay futures and generic relay stream
  • remove App abstraction
  • finer logging
    • on the level of individual relays
  • provide context for more errors to make troubleshooting easier
  • add license headers add license header to every file #180
  • more docstrings
  • pin rust version on travis to not break rustfmt check randomly

This was referenced Apr 11, 2018
Closed
Closed
@snd
Copy link
Contributor Author

snd commented May 2, 2018

still going down the refactoring rabbit hole ; ) lots of local changes still to be committed and pushed. though the end is in sight and i'm much more comfortable with the codebase

@snd snd changed the title refactor (work in progress) refactor&more (work in progress) May 2, 2018
@snd snd mentioned this pull request Jul 2, 2018
Closed
@bjornwgnr bjornwgnr mentioned this pull request Jul 13, 2018
Closed
snd added 21 commits July 17, 2018 13:27
@snd
add bridge/src/log_stream.rs
3a6834f
@snd
add bridge/src/contract_connection.rs
1d8c918
@snd
add web3 dep to cli and deploy
5949012
@snd
refactor
4d49442
@snd
cli/main: improve log message
f3eb56d
@snd
deposit_relay.rs: finalize extracting code for indiviual relay
b8ece04
@snd
log_stream.rs: LogStreamItem -> LogRange
39a3232
@snd
remove unused type CallResult
aa1231e
@snd
pluralize names of relay streams and init deposit relay correctly
75b6ce8
@snd
improve log messages
e8d1d5b
@snd
extract RelayStream abstraction
7e106a2
@snd
make deposit relay use RelayStream
830f23b
@snd
cli/main: remove target from logging
cf3f6cd
@snd
bridge/lib: pub use relay_stream
f69652e
@snd
use RelayStream for all relays, rename, various improvements
f0bf208
@snd
log message for every step of every relay
e05a6e2
@snd
bind futures to vars as prep for adding error context
1dbad82
@snd
decouple RelayStream from LogStream and Bridge from Database
c36959b
@snd
chain_err to add context to errors and various other small improvements
2d24f94
@snd
use U256 for block number, refactor LogStream
572b743
@snd
fix infinite loop in bridge
f11f3df
snd added 24 commits August 15, 2018 18:30
@snd
main_to_side_sign: add test for when already relayed
72838af
@snd
one less magic hex string
b1dc5d7
@snd
mock_transport!: better error message if more actual requests than ex…
5c4ffd5 
…pected
@snd
rename
4c16bd0
@snd
side to main sign: add transport test for when not relayed
88ef793
@snd
side to main sign: add test for when already relayed
8f6c6c8
@snd
remove tests/tests/withdraw_confirm.rs which is now covered by other …
d09e188 
…tests
@snd
add test for side_to_main_signatures if not relayed and responsible
a8219e0
@snd
side to main signatures: add test for when authority not responsible
cb0ce4f
@snd
cargo fmt
2fff377
@snd
side to main signatures: add test for when responsible but already re…
8288329 
…layed
@snd
remove obsolete test
947315a
@snd
remove obsolete log stream tests
a49e082
@snd
remove now obsolete /tests folder
3fc3b7e
@snd
clean up comment describing timeouts and errors
72540a0
@snd
remove add_license.sh which is only useful once
9a492b9 
#159 (comment)
@snd
travis: comment why we explicitely set rust version
0fedf38 
#159 (comment)
@snd
remove unused import
b1439fe 
#159 (comment)
@snd
remove unused import
7c850c6 
#159 (comment)
@snd
remove tests from workspace
12ad432
@snd
remove unused imports
878e9ae
@snd
Cargo.lock: update and remove use of nonofficial web3 repo
57b290e 
#159 (comment)
@snd
add macro_use which is not unused again
9f629b6
@snd
remove unused imports
e30fd4c
@snd
Copy link
Contributor Author

snd commented Aug 22, 2018

almost done here. i'll give my go when all grumbles are fully addressed

debris
debris approved these changes Aug 24, 2018
View reviewed changes
bridge/src/bridge.rs
main_to_side_sign: RelayStream<LogStream<T>, main_to_side_sign::LogToMainToSideSign<T>>,
side_to_main_sign: RelayStream<LogStream<T>, side_to_main_sign::LogToSideToMainSign<T>>,
side_to_main_signatures:
RelayStream<LogStream<T>, side_to_main_signatures::LogToSideToMainSignatures<T>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be in the new line

bridge/src/config.rs
pub contract: ContractConfig,
pub ipc: PathBuf,
pub http: String,
Copy link
Contributor

Choose a reason for hiding this comment

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

can this be configurable? ipc or http?

bridge/src/helpers.rs
}

/// extends the `Stream` trait by the `last` function
pub trait StreamExt<I> {
Copy link
Contributor

Choose a reason for hiding this comment

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

is it used only in tests? if so, can you move it to cfg(test)?

@debris
Copy link
Contributor

debris commented Aug 24, 2018

let's get it in and resolve other issues separately

@debris debris merged commit d75443e into master Aug 24, 2018
@debris debris deleted the snd-refactor branch August 24, 2018 12:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@debris debris debris approved these changes

Assignees
No one assigned
Labels
A0-pleasereview
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

merge home/foreign authority addresses
3 participants
@snd @debris @JosefJ