Skip to content
Permalink
Browse files

Wallet API: remove unused enum Priority from UnsignedTransaction

  • Loading branch information...
stoffu committed Jan 17, 2018
1 parent 09d19c9 commit a9cae0abe7b71c4a112089016cb16ee1ee12d69f
Showing with 0 additions and 7 deletions.
  1. +0 −7 src/wallet/api/wallet2_api.h
@@ -105,13 +105,6 @@ struct UnsignedTransaction
Status_Critical
};

enum Priority {
Priority_Low = 1,
Priority_Medium = 2,
Priority_High = 3,
Priority_Last
};

virtual ~UnsignedTransaction() = 0;
virtual int status() const = 0;
virtual std::string errorString() const = 0;

3 comments on commit a9cae0a

@Neozaru

This comment has been minimized.

Copy link
Contributor

replied Jan 29, 2018

It may not be used in this repository, but it seems to be used at least one time in current master of monero-gui.
https://github.com/monero-project/monero-gui/search?utf8=%E2%9C%93&q=Monero%3A%3AUnsignedTransaction%3A%3APriority_Low%2C&type=

@stoffu

This comment has been minimized.

Copy link
Contributor Author

replied Jan 29, 2018

It was my overlooking. But still, the design of the Wallet API and the GUI's libwalletqt seems quite weird:

  • First of all, the priority should be defined on its own, not dependent on pending transaction or unsigned transaction. Having identical definitions under both PendingTransaction and UnsignedTransaction is redundant and confusing. It should've been defined directly under the Monero namespace. I noticed this redundancy when I had to introduce a new enum Priority_Default = 0 in #3123. Currently, only PendingTransaction::Priority is used in the API, so UnsignedTransaction::Priority was removed here.

  • The GUI's libwalletqt defines both UnsignedTransaction::Priority and PendingTransaction::Priority, and neither of them are used anywhere in the code! It's only referenced in a commented out line (https://github.com/monero-project/monero-gui/blob/master/pages/Transfer.qml#L186). Both of these two definitions seem unnecessary.

I'm not sure if/how this kind of redundancy should be eliminated, though.

@Neozaru

This comment has been minimized.

Copy link
Contributor

replied Jan 29, 2018

I agree that having either Monero::Transaction_Priority or a generic Monero::Transaction to put the enum in may be better. Same reasoning for Status, I guess..

Please sign in to comment.
You can’t perform that action at this time.