-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Unit test that sign_for returns a correct hash (RIPD-1583) #2333
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1134,6 +1134,80 @@ class MultiSign_test : public beast::unit_test::suite | |
env.close(); | ||
} | ||
|
||
void test_signForHash() | ||
{ | ||
// Make sure that the "hash" field returned by the "sign_for" RPC | ||
// command matches the hash returned when that command is sent | ||
// through "submit_multisigned". Make sure that hash also locates | ||
// the transaction in the ledger. | ||
using namespace jtx; | ||
Account const alice {"alice", KeyType::ed25519}; | ||
|
||
Env env(*this); | ||
env.fund (XRP(1000), alice); | ||
env.close(); | ||
|
||
env (signers (alice, 2, {{bogie, 1}, {ghost, 1}})); | ||
env.close(); | ||
|
||
// Use sign_for to sign a transaction where alice pays 10 XRP to | ||
// masterpassphrase. | ||
std::uint32_t baseFee = env.current()->fees().base; | ||
Json::Value jvSig1; | ||
jvSig1[jss::account] = bogie.human(); | ||
jvSig1[jss::secret] = bogie.name(); | ||
jvSig1[jss::tx_json][jss::Account] = alice.human(); | ||
jvSig1[jss::tx_json][jss::Amount] = 10000000; | ||
jvSig1[jss::tx_json][jss::Destination] = env.master.human(); | ||
jvSig1[jss::tx_json][jss::Fee] = 3 * baseFee; | ||
jvSig1[jss::tx_json][jss::Sequence] = env.seq(alice); | ||
jvSig1[jss::tx_json][jss::TransactionType] = "Payment"; | ||
|
||
Json::Value jvSig2 = env.rpc ( | ||
"json", "sign_for", to_string (jvSig1)); | ||
BEAST_EXPECT ( | ||
jvSig2[jss::result][jss::status].asString() == "success"); | ||
|
||
// Save the hash with one signature for use later. | ||
std::string const hash1 = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it worth checking any of the hash characteristics here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Possibly? To me the important characteristic of the hash is it that it allows me to look up the transaction in the ledger. So the various hash checks I'm doing throughout the test show three things:
Are there other characteristics I should be testing for? Thanks. |
||
jvSig2[jss::result][jss::tx_json][jss::hash].asString(); | ||
|
||
// Add the next signature and sign again. | ||
jvSig2[jss::result][jss::account] = ghost.human(); | ||
jvSig2[jss::result][jss::secret] = ghost.name(); | ||
Json::Value jvSubmit = env.rpc ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually used |
||
"json", "sign_for", to_string (jvSig2[jss::result])); | ||
BEAST_EXPECT ( | ||
jvSubmit[jss::result][jss::status].asString() == "success"); | ||
|
||
// Save the hash with two signatures for use later. | ||
std::string const hash2 = | ||
jvSubmit[jss::result][jss::tx_json][jss::hash].asString(); | ||
BEAST_EXPECT (hash1 != hash2); | ||
|
||
// Submit the result of the two signatures. | ||
Json::Value jvResult = env.rpc ( | ||
"json", "submit_multisigned", to_string (jvSubmit[jss::result])); | ||
BEAST_EXPECT ( | ||
jvResult[jss::result][jss::status].asString() == "success"); | ||
BEAST_EXPECT (jvResult[jss::result] | ||
[jss::engine_result].asString() == "tesSUCCESS"); | ||
|
||
// The hash from the submit should be the same as the hash from the | ||
// second signing. | ||
BEAST_EXPECT ( | ||
hash2 == jvResult[jss::result][jss::tx_json][jss::hash].asString()); | ||
env.close(); | ||
|
||
// The transaction we just submitted should now be available and | ||
// validated. | ||
Json::Value jvTx = env.rpc ("tx", hash2); | ||
BEAST_EXPECT (jvTx[jss::result][jss::status].asString() == "success"); | ||
BEAST_EXPECT (jvTx[jss::result][jss::validated].asString() == "true"); | ||
BEAST_EXPECT (jvTx[jss::result][jss::meta] | ||
[sfTransactionResult.jsonName].asString() == "tesSUCCESS"); | ||
} | ||
|
||
void run() override | ||
{ | ||
test_noReserve(); | ||
|
@@ -1152,6 +1226,7 @@ class MultiSign_test : public beast::unit_test::suite | |
test_badSignatureText(); | ||
test_noMultiSigners(); | ||
test_multisigningMultisigner(); | ||
test_signForHash(); | ||
} | ||
}; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only the last
sign_for
before submitting returns the "right" hash huh? In general, each signer doesn't know the hash to look out for though right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. Each added signature changes the hash. But if the transaction with those signatures is submitted, either as a blob through
submit
or as text throughsubmit_multisigned
, then the hash does not change.As a use model, it is probably smartest to rely on the hash returned when the transaction is submitted. But since we are returning a hash value on signing, the value we return should be correct.