Skip to content
This repository has been archived by the owner on Apr 17, 2019. It is now read-only.

* Fix operator== for signatures type in old-fashion model and sh_m #1302

Merged
merged 5 commits into from
May 7, 2018

Conversation

muratovv
Copy link
Contributor

@muratovv muratovv commented May 4, 2018

Description of the Change

  • Fix sh_m and old-fashion model signature operator==
  • Fix ::addSignature for sh_m tx and block

Benefits

Close potential security issues for Iroha's stateless validation.

Usage Examples or Tests [optional]

Tests are provided.

@muratovv muratovv requested review from luckychess and l4l May 4, 2018 14:19
/**
* @given Transaction with marked signature
* @when Invoke ::addSignature with same public key but different signed
* @then Expect that second signature doesn't added
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't -> wasn't

/**
* @given Block with marked signature
* @when Invoke ::addSignature with same public key but different signed
* @then Expect that second signature doesn't added
Copy link
Contributor

Choose a reason for hiding this comment

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

Same (doesn't -> wasn't)

shared_model::crypto::PublicKey("key_one")));
ASSERT_FALSE(block.addSignature(shared_model::crypto::Signed("sign_two"),
shared_model::crypto::PublicKey("key_one")));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Newline plz.

}

/**
* @given Transaction with marked signature
Copy link
Contributor

Choose a reason for hiding this comment

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

I think better to use "given" instead of "marked".
Or may be even move everything about transaction to "when" part (e. g. given transaction, when addSignature() invoked twice with ...)

}

/**
* @given Block with marked signature
Copy link
Contributor

Choose a reason for hiding this comment

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

See note about "marked" from previous test.

Copy link
Contributor

@l4l l4l left a comment

Choose a reason for hiding this comment

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

Imo the test should be in test/module/shared_model/backend_proto

#include "module/shared_model/builders/protobuf/test_transaction_builder.hpp"

/**
* @given Two signatures with same pub key but different signed
Copy link
Contributor

Choose a reason for hiding this comment

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

signed -> signature

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In terms of sm_m objects Signature is a pair: PubKey × Signed. Here I want to reflect different signed explicitly.

* Fix ::addSignature for tx and block

Signed-off-by: Fedor Muratov <muratovfyodor@yandex.ru>
Signed-off-by: Fedor Muratov <muratovfyodor@yandex.ru>
Signed-off-by: Fedor Muratov <muratovfyodor@yandex.ru>
…ransaction

Signed-off-by: Fedor Muratov <muratovfyodor@yandex.ru>
* Fix rebase issues: add const ref for tx and block, also, fix clang-format

Signed-off-by: Fedor Muratov <muratovfyodor@yandex.ru>
@muratovv muratovv merged commit e662fc4 into develop May 7, 2018
@l4l l4l deleted the fix/model_sigantures branch May 11, 2018 12:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants