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

Extend GET swaps/rfc003/:id response + state machine re-architecting #752

Merged
merged 10 commits into from Feb 18, 2019

Conversation

@luckysori
Copy link
Contributor

luckysori commented Feb 11, 2019

The original goal of this PR was to extend the body of the response when sending a get request to /swaps/rfc003/:id, as per #693. When attempting to do this, we realised that it would be very difficult to do with the way we store state machine states directly. Therefore the scope of this PR has changed to include part of the famous re-architecting of the state machine, as discussed here #576.

Fixes #693.

@wafflebot wafflebot bot added the review label Feb 11, 2019

@mergify mergify bot added work-in-progress and removed review labels Feb 11, 2019

@luckysori luckysori changed the title [WIP] [WIP] Extend GET swaps/rfc003/:id response + state machine re-architecting Feb 11, 2019

@thomaseizinger
Copy link
Member

thomaseizinger left a comment

Super awesome exciting work guys! 🎉🎉🎉

Show resolved Hide resolved application/comit_node/src/http_api/rfc003/swap.rs Outdated
Show resolved Hide resolved application/comit_node/src/swap_protocols/rfc003/alice/actions/btc_erc20.rs Outdated
Show resolved Hide resolved application/comit_node/src/swap_protocols/rfc003/bitcoin/mod.rs
Show resolved Hide resolved application/comit_node/src/swap_protocols/rfc003/bob/actions/mod.rs Outdated
SwapCommunication::Proposed { response, .. } => {
Some(Box::new(response.response_future()))
}
_ => None,

This comment has been minimized.

@thomaseizinger

thomaseizinger Feb 11, 2019

Member

🔧 We could log a warn here with the current state because calling this in any state other than Proposed will likely yield an error and then this message might be a good hint to discover why.

Show resolved Hide resolved application/comit_node/src/swap_protocols/rfc003/bob/spawner.rs Outdated
Show resolved Hide resolved application/comit_node/src/swap_protocols/rfc003/messages.rs Outdated
Show resolved Hide resolved application/comit_node/src/swap_protocols/rfc003/messages.rs Outdated
Show resolved Hide resolved application/comit_node/src/swap_protocols/rfc003/state_machine.rs

@wafflebot wafflebot bot added review and removed work-in-progress labels Feb 11, 2019

@mergify mergify bot added work-in-progress and removed review labels Feb 11, 2019

{
fn from(request: rfc003::messages::Request<AL, BL, AA, BA>) -> Self {
Self {
alpha_ledger: request.alpha_ledger.to_http_ledger().unwrap(),

This comment has been minimized.

@thomaseizinger

thomaseizinger Feb 11, 2019

Member

🤔 Should we really unwrap here?

This comment has been minimized.

@luckysori

luckysori Feb 15, 2019

Author Contributor

Clippy doesn't let you anyway, so I replaced it with a new().

@wafflebot wafflebot bot added review and removed work-in-progress labels Feb 11, 2019

@mergify mergify bot added work-in-progress and removed review labels Feb 11, 2019

@wafflebot wafflebot bot added review and removed work-in-progress labels Feb 12, 2019

@mergify mergify bot added work-in-progress and removed review labels Feb 12, 2019

@wafflebot wafflebot bot added review and removed work-in-progress labels Feb 12, 2019

@mergify mergify bot added work-in-progress and removed review labels Feb 12, 2019

@wafflebot wafflebot bot added review and removed work-in-progress labels Feb 12, 2019

@mergify mergify bot added work-in-progress and removed review labels Feb 12, 2019

@wafflebot wafflebot bot removed the work-in-progress label Feb 12, 2019

@wafflebot wafflebot bot added review and removed work-in-progress labels Feb 15, 2019

@mergify mergify bot added work-in-progress and removed review labels Feb 15, 2019

@luckysori luckysori added review and removed work-in-progress labels Feb 15, 2019

@luckysori luckysori changed the title [WIP] Extend GET swaps/rfc003/:id response + state machine re-architecting Extend GET swaps/rfc003/:id response + state machine re-architecting Feb 15, 2019

@luckysori luckysori force-pushed the 693-extend-get-swap-response branch from 2ebc1ee to ee254f2 Feb 18, 2019

@thomaseizinger thomaseizinger force-pushed the 693-extend-get-swap-response branch 2 times, most recently from fa31d14 to 6cc6ea0 Feb 18, 2019

@luckysori luckysori referenced this pull request Feb 18, 2019

Open

Missing HTTP API tests #766

0 of 2 tasks complete
@thomaseizinger
Copy link
Member

thomaseizinger left a comment

Looks good from my side :)

Lucas Soriano and others added some commits Feb 1, 2019

Lucas Soriano mergify-bot
Rearchitect storage + expand GET /swaps/rfc003/:id
- Return full swap details as per #497 spike results.
- Remove lock duration completely, which should have disappeared in a
  previous PR.
- State store now stores `alice::State`s and `bob::State`s, which
  include more information about the swap and can be more easily
  extended or modified in the future without having to alter the state
  machine that much. Saving the deploy and fund transactions for a
  HTLC is not yet supported.
- Remove seed from HTTP API. The swap seed is now stored inside
  `alice::State` and `bob::State` and it acts as both the secret
  source for Alice, and a Bitcoin redeem and refund address generator
  for both Alice and Bob.
- Emit state machine updates through `mpsc::unbounded()` channel.
  State machine states are sent across threads in a streaming fashion
  from the state machine to the state machine spawner. These are then
  passed on to the storage layer, which translates them and updates
  the corresponding `alice::State` or `bob::State`.
- Deny unknown fields in swap request body.
- Reduce e2e tests to only check for availability of actions to
  execute them. Therefore, the correctness of the HTTP API should be
  tested separately.
Lucas Soriano mergify-bot
Fix race condition on Travis
- Generalise `poll_comit_node_until_actions` into
  `poll_comit_node_until`, since `poll_comit_node_until` was extended
  to handle any kind of response.
Get rid of SwapAccepted type
We inline rfc003::Request into the SwapCommunication:Accepted variant
and use the AcceptResponseBody type to model the remaining
information.
Remove TODO regarding Refunded
Will be mentioned in the issue

@D4nte D4nte force-pushed the 693-extend-get-swap-response branch from 27d816e to f9bbecf Feb 18, 2019

@mergify mergify bot merged commit e0aaad7 into master Feb 18, 2019

1 check passed

license/cla Contributor License Agreement is signed.
Details

@mergify mergify bot deleted the 693-extend-get-swap-response branch Feb 18, 2019

@wafflebot wafflebot bot removed the review label Feb 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.