Made use of get_first_public_key to always return a public key even i… #280

prix-uniris · 2022-04-18T16:55:34Z

Made use of TransactionChain.get_first_public_key/1 to always return a public key even in case of nonexistent transactions and lower I/O operations which are not required if only want to get First_Public_Key for any given address.

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Test A
Test B

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

…n case of non existent transactions and lower I/O operations which are not required if only want to get First_Public_Key for any given address

prix-uniris · 2022-04-18T16:56:55Z

This Fixes Issue #276

ghost · 2022-04-19T07:12:58Z

lib/archethic/p2p/message.ex

@@ -1227,12 +1227,9 @@ defmodule ArchEthic.P2P.Message do
  end

  def process(%GetFirstPublicKey{address: address}) do


We should also rename address to public_key, don't you think ?

Sure sam, that makes more sense.

but we are supplying address to get a public key for a given address right

so address also is a better argument name, also %GetFirstPublicKey{} accepts a address: , so variable name of address make more sense.

Hi, please keep it as public key, and in comments put address explaining it .

The name is misleading the purpose of the function.
So in GetFirstPublicKey, the public key should have more meaning.
Moreover, the implementation underneath is not using the address at all

Okay thanks, I think it's more clear now

I have made the changes.

apoorv-2204

https://misc.archethic.net/s/gbdQ_8L_a

ghost · 2022-04-20T07:13:18Z

I think you can rename the field in the struct get_first_public_key.ex

prix-uniris · 2022-04-20T08:11:25Z

Thanks sam, changed that too and in relevant places.

ghost · 2022-04-20T08:15:42Z

lib/archethic/p2p/message.ex

@@ -706,10 +706,10 @@ defmodule ArchEthic.P2P.Message do
  end

  def decode(<<20::8, rest::bitstring>>) do
-    {address, rest} = Utils.deserialize_address(rest)
+    {public_key, rest} = Utils.deserialize_address(rest)


You should use Utils.deserialize_public_key here

ghost · 2022-04-20T08:17:56Z

lib/archethic/p2p/message.ex

-        %NotFound{}
+  # Returns the first public_key for a given public_key and if the public_key is used for the first time, return the same public_key.
+  def process(%GetFirstPublicKey{public_key: public_key}) do
+    case TransactionChain.get_first_public_key(public_key) do


I think the case statement is not required anymore, as you are expecting a single result

so should we just directly return it as return value is always expected

Thanks, that makes more sense to directly return

…xpected from the function and changed decode function from `deserialize_address` to `deserialize_public_key`

prix-uniris · 2022-04-20T08:24:57Z

Made the above mentioned changes.

* Made use of get_first_public_key to always return a public key even in case of non existent transactions and lower I/O operations which are not required if only want to get First_Public_Key for any given address * Renamed `address` to `public_key` as it is more meaningful * Changed Struct atom of GetFirstPublicKey to `public_key` from `address` * Removed case statement from %GetFirstPublicKey as a value is always expected from the function and changed decode function from `deserialize_address` to `deserialize_public_key`

Made use of get_first_public_key to always return a public key even i…

d4b33a8

…n case of non existent transactions and lower I/O operations which are not required if only want to get First_Public_Key for any given address

prix-uniris requested a review from apoorv-2204 April 18, 2022 16:55

ghost reviewed Apr 19, 2022

View reviewed changes

apoorv-2204 approved these changes Apr 19, 2022

View reviewed changes

Renamed address to public_key as it is more meaningful

d956389

Changed Struct atom of GetFirstPublicKey to public_key from address

201cae0

ghost reviewed Apr 20, 2022

View reviewed changes

Removed case statement from %GetFirstPublicKey as a value is always e…

99885e5

…xpected from the function and changed decode function from `deserialize_address` to `deserialize_public_key`

ghost approved these changes Apr 21, 2022

View reviewed changes

ghost added bug Something isn't working P2P Involve P2P networking serialization Involve message serialization labels Apr 21, 2022

ghost merged commit 2879c2f into archethic-foundation:develop Apr 21, 2022

ghost mentioned this pull request Apr 27, 2022

improve mining error handling #289

Merged

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Made use of get_first_public_key to always return a public key even i… #280

Made use of get_first_public_key to always return a public key even i… #280

prix-uniris commented Apr 18, 2022

prix-uniris commented Apr 18, 2022

ghost Apr 19, 2022

prix-uniris Apr 19, 2022

prix-uniris Apr 19, 2022

prix-uniris Apr 19, 2022

apoorv-2204 Apr 19, 2022

ghost Apr 19, 2022 •

edited by ghost

Loading

prix-uniris Apr 20, 2022

prix-uniris Apr 20, 2022

apoorv-2204 left a comment

ghost commented Apr 20, 2022

prix-uniris commented Apr 20, 2022

ghost Apr 20, 2022

ghost Apr 20, 2022

prix-uniris Apr 20, 2022

ghost Apr 20, 2022

prix-uniris Apr 20, 2022

prix-uniris commented Apr 20, 2022

		@@ -1227,12 +1227,9 @@ defmodule ArchEthic.P2P.Message do
		end

		def process(%GetFirstPublicKey{address: address}) do

Made use of get_first_public_key to always return a public key even i… #280

Made use of get_first_public_key to always return a public key even i… #280

Conversation

prix-uniris commented Apr 18, 2022

Description

Type of change

How Has This Been Tested?

Checklist:

prix-uniris commented Apr 18, 2022

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

ghost Apr 19, 2022 • edited by ghost Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apoorv-2204 left a comment

Choose a reason for hiding this comment

ghost commented Apr 20, 2022

prix-uniris commented Apr 20, 2022

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

prix-uniris commented Apr 20, 2022

ghost Apr 19, 2022 •

edited by ghost

Loading