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.

Sign up for GitHub

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

Add DeliverMax (alias for Amount) in Payment transactions #4733

Merged
merged 26 commits into from
Oct 23, 2023
Merged

Add DeliverMax (alias for Amount) in Payment transactions #4733

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
a4adde5
Add DeliverMax to submit, sign, tx etc.
Bronek Sep 26, 2023
7d79f89
Add DeliverMax to subscribe
Bronek Sep 28, 2023
c9f7907
Factor out MultivarJson for transaction streams
Bronek Sep 28, 2023
775731b
Refactor transJson to remove repeated code
Bronek Oct 5, 2023
2959308
Add unit test for MultivarJson
Bronek Oct 6, 2023
ef6cbc0
Add test for TransactionSign.cpp
Bronek Oct 6, 2023
2ec189b
Add DeliverMax to transaction_entry tests
Bronek Oct 9, 2023
aa9f0a9
Add DeliverMax to subscribe tests
Bronek Oct 9, 2023
e35bf62
Allow both Amount and DeliverMax if identical
Bronek Oct 9, 2023
03edc89
Add DeliverMax to tx tests
Bronek Oct 10, 2023
9f4206a
Add DeliverMax to account_tx test
Bronek Oct 10, 2023
432fd16
Change apiVersion to unsigned
Bronek Oct 10, 2023
03713c1
Merge branch 'develop' into feature/partial-payment-fieldname
Bronek Oct 11, 2023
c99959e
Update API-CHANGELOG.md
Bronek Oct 11, 2023
d1a2c59
Merge branch 'develop' into feature/partial-payment-fieldname
Bronek Oct 16, 2023
44771d1
Merge branch 'develop' into feature/partial-payment-fieldname
Bronek Oct 18, 2023
48697e8
Minor fixes
Bronek Oct 19, 2023
ff41b90
Add MultivarJson::isMember and small fixes
Bronek Oct 19, 2023
5b30a25
Fill MultiApiJson in NetworkOPsImp::transJson
Bronek Oct 19, 2023
198e7cd
Merge branch 'develop' into feature/partial-payment-fieldname
Bronek Oct 20, 2023
bd53535
Minor test improvement
Bronek Oct 20, 2023
cf0798a
Add assert in MultivarJson::select
Bronek Oct 20, 2023
efbf39a
Minor improvement in unit tests
Bronek Oct 23, 2023
70bd1b7
Minor cleanup in include guards
Bronek Oct 23, 2023
5e7fa7c
Improve version loop and static asserts
Bronek Oct 23, 2023
f4bfe95
Merge branch 'develop' into feature/partial-payment-fieldname
intelliot Oct 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Builds/levelization/results/ordering.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ test.csf > ripple.protocol
test.csf > test.jtx
test.json > ripple.beast
test.json > ripple.json
test.json > ripple.rpc
Copy link
Collaborator

Choose a reason for hiding this comment

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

Adding lines to the ordering file is great, as long as it follows the existing rules (which this does). It would be a problem if the line was added to loops.txt, or if the dependency didn't "fit" the existing order.

test.json > test.jtx
test.jtx > ripple.app
test.jtx > ripple.basics
Expand Down
4 changes: 2 additions & 2 deletions src/ripple/app/misc/DeliverMax.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
*/
//==============================================================================

#ifndef RIPPLE_RPC_DELIVERMAX_H_INCLUDED
#define RIPPLE_RPC_DELIVERMAX_H_INCLUDED
#ifndef RIPPLE_APP_MISC_DELIVERMAX_H_INCLUDED
#define RIPPLE_APP_MISC_DELIVERMAX_H_INCLUDED

#include <ripple/protocol/TxFormats.h>

Expand Down
13 changes: 9 additions & 4 deletions src/ripple/app/misc/NetworkOPs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3150,10 +3150,15 @@ NetworkOPsImp::transJson(
}

MultiApiJson multiObj({jvObj, jvObj});
static_assert(MultiApiJson::size == 2);
static_assert(RPC::apiMinimumSupportedVersion == 1);
for (unsigned apiVersion = 1, lastIndex = MultiApiJson::size;
apiVersion <= 2;
// Minimum supported API version must match index 0 in MultiApiJson
static_assert(apiVersionSelector(RPC::apiMinimumSupportedVersion)() == 0);
// Beta API version must match last index in MultiApiJson
static_assert(
apiVersionSelector(RPC::apiBetaVersion)() + 1 //
== MultiApiJson::size);
for (unsigned apiVersion = RPC::apiMinimumSupportedVersion,
Copy link
Collaborator

@arihantkothari arihantkothari Oct 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

For the intentions of looping over the apiVersions from minimumSupported to beta. I actually made a helper function long time back - may/ may not be useful :) [Now I feel, that the helper can be improved and is too strict. But I'll leave as it is.]
#4611

Copy link
Collaborator

Choose a reason for hiding this comment

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

That function is cool, but as written, it's only for tests.

Copy link
Collaborator

@arihantkothari arihantkothari Oct 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

oh shoot, my bad - sorry about that. I did not notice this was not for tests 😆

Copy link
Collaborator

Choose a reason for hiding this comment

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

No worries

lastIndex = MultiApiJson::size;
apiVersion <= RPC::apiBetaVersion;
++apiVersion)
{
unsigned const index = apiVersionSelector(apiVersion)();
Expand Down
4 changes: 2 additions & 2 deletions src/ripple/json/MultivarJson.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
*/
//==============================================================================

#ifndef RIPPLE_BASICS_MULTIVARJSON_H_INCLUDED
#define RIPPLE_BASICS_MULTIVARJSON_H_INCLUDED
#ifndef RIPPLE_JSON_MULTIVARJSON_H_INCLUDED
#define RIPPLE_JSON_MULTIVARJSON_H_INCLUDED

#include <ripple/json/json_value.h>

Expand Down
15 changes: 15 additions & 0 deletions src/test/json/MultivarJson_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
//==============================================================================

#include <ripple/json/MultivarJson.h>
#include <ripple/rpc/impl/RPCHelpers.h>

#include <ripple/beast/unit_test.h>
#include "ripple/beast/unit_test/suite.hpp"
Expand Down Expand Up @@ -244,6 +245,7 @@ struct MultivarJson_test : beast::unit_test::suite
}

{
// NOTE It's fine to change this test when we change API versions
testcase("apiVersionSelector");

static_assert(MultiApiJson::size == 2);
Expand All @@ -269,6 +271,19 @@ struct MultivarJson_test : beast::unit_test::suite
apiVersionSelector(
std::numeric_limits<unsigned int>::max())() == 1);
}

{
// There should be no reson to change this test
testcase("apiVersionSelector invariants");

static_assert(
apiVersionSelector(RPC::apiMinimumSupportedVersion)() == 0);
static_assert(
apiVersionSelector(RPC::apiBetaVersion)() + 1 //
== MultiApiJson::size);

BEAST_EXPECT(MultiApiJson::size >= 1);
}
}
};

Expand Down
10 changes: 3 additions & 7 deletions src/test/rpc/Transaction_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -737,16 +737,12 @@ class Transaction_test : public beast::unit_test::suite
if (BEAST_EXPECT(result[jss::result].isMember(name)))
{
auto const received = result[jss::result][name];
std::ostringstream ssReceived;
ssReceived << received;
std::ostringstream ssExpected;
ssExpected << *memberIt;
BEAST_EXPECTS(
received == *memberIt,
"Transaction contains \n\"" + name + "\": " //
+ ssReceived.str() //
+ "but expected " //
+ ssExpected.str());
+ to_string(received) //
+ " but expected " //
+ to_string(expected));
}
}
}
Expand Down
Loading