REM-1312: Get particular Atomic Swap information by its identifier #24

anastasiia-bilova · 2019-04-22T15:51:34Z

Jira references

Ticket (task) — https://remmeio.atlassian.net/browse/REM-1312

Description

Get information about Atomic Swap command line interface's command implementation.

Arguments	Type	Required	Description
id	String	Yes	Swap identifier to get an information about swap by.
node-url	String	No	Node URL to apply a command to.

Example of the usage:

$ remme atomic-swap get-info \
      --id=033402fe1346742486b15a3a9966eb5249271025fc7fb0b37ed3fdb4bcce6808 \
      --node-url=node-genesis-testnet.remme.io
{
    "result": {
        "information": {
            "amount": "10.0000",
            "created_at": 1556803765,
            "email_address_encrypted_optional": "",
            "is_initiator": false,
            "receiver_address": "112007484def48e1c6b77cf784aeabcac51222e48ae14f3821697f4040247ba01558b1",
            "secret_key": "",
            "secret_lock": "0728356568862f9da0825aa45ae9d3642d64a6a732ad70b8857b2823dbf2a0b8",
            "sender_address": "1120076ecf036e857f42129b58303bcf1e03723764a1702cbe98529802aad8514ee3cf",
            "sender_address_non_local": "0xe6ca0e7c974f06471759e9a05d18b538c5ced11e",
            "state": "OPENED",
            "swap_id": "033402fe1346742486b15a3a9966eb5249271025fc7fb0b37ed3fdb4bcce6808"
        }
    }
}

Implemented

— Command line interface's command.
— Service to get particular Atomic Swap information of the atomic swap class.

References

CLI framework — https://click.palletsprojects.com/en/7.x
Library to interact with Remme-core — https://github.com/Remmeauth/remme-client-python
Custom Marshmellow fields — https://marshmallow.readthedocs.io/en/3.0/custom_fields.html

…t-atomic-swap-info-by-its-id

…by swap identifier

codecov-io · 2019-04-22T16:28:56Z

Codecov Report

Merging #24 into develop will decrease coverage by <.01%.
The diff coverage is 97.5%.

@@             Coverage Diff             @@
##           develop      #24      +/-   ##
===========================================
- Coverage    97.43%   97.43%   -0.01%     
===========================================
  Files           29       30       +1     
  Lines          507      545      +38     
===========================================
+ Hits           494      531      +37     
- Misses          13       14       +1

Impacted Files	Coverage Δ
cli/atomic_swap/service.py	`100% <100%> (ø)`	⬆️
cli/atomic_swap/help.py	`100% <100%> (ø)`
cli/constants.py	`100% <100%> (ø)`	⬆️
cli/atomic_swap/cli.py	`100% <100%> (ø)`	⬆️
cli/atomic_swap/forms.py	`100% <100%> (ø)`	⬆️
cli/generic/forms/fields.py	`100% <100%> (ø)`	⬆️
cli/atomic_swap/interfaces.py	`60% <50%> (-6.67%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 666eb09...881434c. Read the comment docs.

…t-atomic-swap-info-by-its-id

dmytrostriletskyi · 2019-05-03T10:47:29Z

README.md

@@ -158,6 +159,36 @@ $ remme atomic-swap get-public-key --node-url=node-6-testnet.remme.io
 }
 ```

+Get information about atomic swap by swap identifier — ``remme atomic-swap get-info``:


by swap identifier > by its identifier.

@dmytrostriletskyi, done.

dmytrostriletskyi · 2019-05-03T10:47:56Z

cli/atomic_swap/cli.py

@@ -18,6 +23,8 @@
    print_result,
 )

+loop = asyncio.get_event_loop()


You do not need a loop in the CLI layer.

@dmytrostriletskyi, done.

dmytrostriletskyi · 2019-05-03T10:49:24Z

cli/generic/forms/fields.py

@@ -102,3 +103,23 @@ def _deserialize(self, value, attr, data, **kwargs):
            raise ValidationError(f'The following public key address `{public_key_address}` is invalid.')

        return value
+
+
+class SwapIdField(fields.Field):


SwapIdField > SwapIdentifierField.

@dmytrostriletskyi, done.

dmytrostriletskyi · 2019-05-03T10:56:18Z

cli/atomic_swap/forms.py

+class GetAtomicSwapInformationForm(Schema):
+    """
+    Get information about atomic swap of the atomic swap form.
+    """


Get information about atomic swap by its identifier form.

@dmytrostriletskyi, done.

…t-atomic-swap-info-by-its-id

Alladin9393 · 2019-05-03T12:48:09Z

cli/atomic_swap/service.py

+        try:
+            swap_info = loop.run_until_complete(self.service.swap.get_info(swap_id=swap_id))
+
+        except RpcGenericServerDefinedError as error:


Add except RpcGenericServerDefinedError to method get_public_key.

This ticket is not related to get_public_key. It should be fixed in another ticket.

Alladin9393 · 2019-05-03T12:53:13Z

cli/generic/forms/fields.py

+        """
+        Validate data (swap identifier) that was passed to field.
+        """
+        swap_identifier = value


What is it for?

For more detail, variable value says nothing.

You create a class that named SwapIdentifierField that have docs that say

Implements validation of the swap identifier.

and you have a method _deserialize that also have docs:

Validate data (swap identifier) that was passed to field.

Marshmallow

Example of creating a field

Also, you validate one variable and return another, it not logical.

@Alladin9393,

Documentation could lag behind code during the project life.

Sometimes programmers read the code but not the docs.

These points are the core arguments why we should create the separated explicit (!) variable and return it.

@dmytrostriletskyi, ok, but we don't return swap_identifier, we validate one argument, but return other value.

@Alladin9393, refresh page.

Alladin9393 · 2019-05-03T13:01:18Z

tests/atomic_swap/test_get_info.py

+    swap_info = json.loads(result.output).get('result').get('information')
+
+    assert PASSED_EXIT_FROM_COMMAND_CODE == result.exit_code
+    assert re.match(pattern=ADDRESS_REGEXP, string=swap_info.get('sender_address')) is not None


Why you check just pattern? Check, please equality swap_id or if swap_id in data. Add/Replace more one checking.

@Alladin9393, done.

Alladin9393 · 2019-05-03T13:14:46Z

cli/constants.py

@@ -7,6 +7,7 @@
 PUBLIC_KEY_REGEXP = r'^[0-9a-f]{66}$'
 PUBLIC_KEY_ADDRESS_REGEXP = r'^[0-9a-f]{70}$'
 HEADER_SIGNATURE_REGEXP = r'^[0-9a-f]{128}$'
+SWAP_IDENTIFIER_REGEXP = r'^[a-f0-9]{64}$'


Change according to alphabetical order.

This constants not in alphabetical order. It should be fixed in another ticket for technical dept.

Alladin9393 · 2019-05-03T14:13:57Z

cli/atomic_swap/help.py

@@ -1,3 +1,4 @@
 """
 Provide help messages for command line interface's atomic swap commands.
 """
+SWAP_IDENTIFIER_ARGUMENT_HELP_MESSAGE = 'Swap identifier to get an information about swap by.'


get information about swap by..

@Alladin9393, done.

Alladin9393 · 2019-05-03T14:17:31Z

tests/atomic_swap/test_get_info.py

+def test_get_swap_info():
+    """
+    Case: get information about atomic swap by its identifier.
+    Expect: information about swap is returned.


the swap is returned..

@Alladin9393, done.

Alladin9393 · 2019-05-03T14:27:08Z

tests/atomic_swap/test_get_info.py

+    assert re.match(pattern=ADDRESS_REGEXP, string=swap_info.get('sender_address')) is not None
+    assert re.match(pattern=ADDRESS_REGEXP, string=swap_info.get('receiver_address')) is not None
+    assert re.match(pattern=SWAP_IDENTIFIER_REGEXP, string=swap_identifier) is not None
+    assert isinstance(int(float(swap_info.get('amount'))), int)


What is this test here for? We will always have an int.

@Alladin9393, done.

Alladin9393 · 2019-05-03T14:31:00Z

tests/atomic_swap/test_get_info.py

+def test_get_swap_info_invalid_swap_id():
+    """
+    Case: get information about atomic swap by its invalid identifier.
+    Expect: the following swap identifier is invalid error message.


swap identifier is an invalid error message..

…t-atomic-swap-info-by-its-id

Alladin9393 · 2019-05-06T11:18:54Z

tests/atomic_swap/test_get_info.py

+
+def test_get_swap_info_non_existing_swap_id():
+    """
+    Case: get information about atomic swap by passing non-existing identifier.


Suggested change

Case: get information about atomic swap by passing non-existing identifier.

Case: get information about atomic swap by passing a non-existing identifier.

Technical dept.

anastasiia-bilova added 2 commits April 22, 2019 14:32

Create basic functionality for atomic swap information

02a8bca

Merge branch 'develop' of github.com:Remmeauth/remme-core-cli into ge…

e628ecd

…t-atomic-swap-info-by-its-id

anastasiia-bilova force-pushed the get-atomic-swap-info-by-its-id branch 2 times, most recently from 4402a5c to e2c2a90 Compare April 22, 2019 16:18

Create command line interface for method get atomic swap information …

a36b469

…by swap identifier

anastasiia-bilova force-pushed the get-atomic-swap-info-by-its-id branch from e2c2a90 to a36b469 Compare April 22, 2019 16:24

anastasiia-bilova added 3 commits May 2, 2019 14:09

Merge branch 'develop' of github.com:Remmeauth/remme-core-cli into ge…

9c2b446

…t-atomic-swap-info-by-its-id

Refactor tests and documentation, create swap_info fixture

4704b21

Merge branch 'develop' of github.com:Remmeauth/remme-core-cli into ge…

cbf1be3

…t-atomic-swap-info-by-its-id

anastasiia-bilova requested a review from Annitra May 2, 2019 13:47

Annitra approved these changes May 2, 2019

View reviewed changes

anastasiia-bilova requested a review from dmytrostriletskyi May 3, 2019 10:06

dmytrostriletskyi suggested changes May 3, 2019

View reviewed changes

anastasiia-bilova force-pushed the get-atomic-swap-info-by-its-id branch from 7cd74a7 to e492667 Compare May 3, 2019 10:53

dmytrostriletskyi approved these changes May 3, 2019

View reviewed changes

Merge branch 'develop' of github.com:Remmeauth/remme-core-cli into ge…

a92a23d

…t-atomic-swap-info-by-its-id

anastasiia-bilova force-pushed the get-atomic-swap-info-by-its-id branch from e492667 to a92a23d Compare May 3, 2019 11:02

anastasiia-bilova requested a review from Alladin9393 May 3, 2019 11:07

Alladin9393 suggested changes May 3, 2019

View reviewed changes

anastasiia-bilova force-pushed the get-atomic-swap-info-by-its-id branch from 306e861 to 6be49ab Compare May 3, 2019 13:54

dmytrostriletskyi mentioned this pull request May 3, 2019

Technical debt of the project #22

Open

34 tasks

Alladin9393 suggested changes May 3, 2019

View reviewed changes

Remmeauth deleted a comment from Alladin9393 May 3, 2019

anastasiia-bilova force-pushed the get-atomic-swap-info-by-its-id branch from 6be49ab to a5ff6f0 Compare May 3, 2019 14:47

Add check for test case get swap info, refactor readme

cef8365

anastasiia-bilova force-pushed the get-atomic-swap-info-by-its-id branch from a5ff6f0 to cef8365 Compare May 3, 2019 14:48

Merge branch 'develop' of github.com:Remmeauth/remme-core-cli into ge…

881434c

…t-atomic-swap-info-by-its-id

Alladin9393 approved these changes May 6, 2019

View reviewed changes

anastasiia-bilova merged commit f7852ff into develop May 6, 2019

delete-merged-branch bot deleted the get-atomic-swap-info-by-its-id branch May 6, 2019 12:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

REM-1312: Get particular Atomic Swap information by its identifier #24

REM-1312: Get particular Atomic Swap information by its identifier #24

anastasiia-bilova commented Apr 22, 2019 •

edited

Loading

codecov-io commented Apr 22, 2019 •

edited

Loading

dmytrostriletskyi May 3, 2019

anastasiia-bilova May 3, 2019

dmytrostriletskyi May 3, 2019

anastasiia-bilova May 3, 2019

dmytrostriletskyi May 3, 2019

anastasiia-bilova May 3, 2019

dmytrostriletskyi May 3, 2019

anastasiia-bilova May 3, 2019

Alladin9393 May 3, 2019

anastasiia-bilova May 3, 2019

Alladin9393 May 3, 2019

anastasiia-bilova May 3, 2019

Alladin9393 May 3, 2019

dmytrostriletskyi May 3, 2019 •

edited

Loading

Alladin9393 May 3, 2019

anastasiia-bilova May 3, 2019

Alladin9393 May 3, 2019

anastasiia-bilova May 3, 2019

Alladin9393 May 3, 2019

anastasiia-bilova May 3, 2019

Alladin9393 May 3, 2019

anastasiia-bilova May 3, 2019

Alladin9393 May 3, 2019

anastasiia-bilova May 3, 2019

Alladin9393 May 3, 2019

anastasiia-bilova May 3, 2019

Alladin9393 May 3, 2019

anastasiia-bilova May 3, 2019

Alladin9393 May 6, 2019

anastasiia-bilova May 6, 2019

	Case: get information about atomic swap by passing non-existing identifier.
	Case: get information about atomic swap by passing a non-existing identifier.

REM-1312: Get particular Atomic Swap information by its identifier #24

REM-1312: Get particular Atomic Swap information by its identifier #24

Conversation

anastasiia-bilova commented Apr 22, 2019 • edited Loading

Jira references

Description

Implemented

References

codecov-io commented Apr 22, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmytrostriletskyi May 3, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anastasiia-bilova commented Apr 22, 2019 •

edited

Loading

codecov-io commented Apr 22, 2019 •

edited

Loading

dmytrostriletskyi May 3, 2019 •

edited

Loading