Save slate participant messages in database #2441
Conversation
yeastplume
changed the title
…sts for serialization functions
| @@ -30,3 +30,6 @@ chrono = "0.4.4" | |||
|
|
|||
| grin_keychain = { path = "../keychain", version = "1.0.0" } | |||
| grin_util = { path = "../util", version = "1.0.0" } | |||
|
|
|||
| [dev-dependencies] | |||
There was a problem hiding this comment.
Anyone have any major problems with this dev-dependency? Would rather avoid but seems best way to run serialization tests.
yeastplume
changed the title
|
Think this is ready to begin review now. |
| @@ -640,6 +643,14 @@ where | |||
| None => w.parent_key_id(), | |||
| }; | |||
|
|
|||
| let message = match message { | |||
There was a problem hiding this comment.
Nitpick: I'd suggest to introduce SlateMessage type and define semantic (it could be None, if not String has limited length) there, we truncate twice in this pr, could forget to do it introducing some new logic later on
wallet/src/libwallet/internal/tx.rs
Outdated
| } | ||
| let mut batch = wallet.batch()?; | ||
| for tx in tx_vec.into_iter() { | ||
| let mut t = tx.clone(); |
There was a problem hiding this comment.
why do we need t here? we consume tx_vec, could we use tx?
wallet/src/libwallet/internal/tx.rs
Outdated
| for tx in tx_vec.into_iter() { | ||
| let mut t = tx.clone(); | ||
| t.messages = Some(slate.participant_messages()); | ||
| batch.save_tx_log_entry(t.clone(), &t.parent_key_id.clone())?; |
There was a problem hiding this comment.
do we need to clone t here?
| t.messages = Some(slate.participant_messages()); | ||
| batch.save_tx_log_entry(t.clone(), &t.parent_key_id.clone())?; | ||
| } | ||
| batch.commit()?; |
There was a problem hiding this comment.
I'd replace 2 lines with just batch.commit();
|
Actually enforcing string length using type system may not be that great idea after all, too many underlying levels must be changed |
|
Thanks for review, tightened up loop a bit. I'd started trying to enforce the message length a bit further down, but ran into issues as you say. So long as it's enforced at the API level (which is what most people will use, I think it should be okay). |
|
@yeastplume is there a way to identify which message belongs to the sender? Or is it always the first one in the array? |
Alternative to #2409. Note that this stores messages within a struct in the database, and the actual formatting (if you want in in JSON, for instance,) can be applied somewhere higher up in the pipeline.
Messages + keys + sig for an output are displayed when showing details of a particular transaction, e.g.
grin wallet txs -i 32.Shouldn't break backwards compat, as the new stored message field in the TxLogEntry is Optional.
Messages are now limited to 256 (unicode) characters and will are truncated at the API level.
Todos: