Add RPCs to bookkeeper so you can update/set a description on an event #7604
Conversation
| lowest = fee->timestamp; | ||
| db_begin_transaction(db); | ||
| edit_utxo_description(db, outpoint, new_desc); | ||
| chain_events = get_chain_events_by_outpoint(cmd, db, outpoint, true); |
There was a problem hiding this comment.
Is returning the events with updated descriptions meant as confirmation that the command was successful and rows were updated? How about failing the command with an error (eg no events found for description) if no rows were updated? Would make it a lot easier to handle error states when using the command via RPC or gRPC. If there is a need to show the results, querying events for an id is easy enough with #7536
There was a problem hiding this comment.
Yes, returning the results is a convenience in case you need/want to display the result to a user in an interface. It also allows for the caller to locally update their stored set of events instead of re-querying.
I go back on forth on whether or not to error on "there's nothing to update that matches that description". Returning an empty result set seems like a fine way of signaling that no results were found -- nothing got updated? Really a matter of taste, and I just so happen to dislike "noisy errors" -> nothing wrong happened during execution it just so happens that the input you put in had no effect.
There was a problem hiding this comment.
I just so happen to dislike "noisy errors" -> nothing wrong happened during execution it just so happens that the input you put in caused no effect
The explicit intention of the command is that something happens so imho nothing happening is an error. But returning an empty result is probably more in line with other CLN commands. As a caller you just have to be careful to check for the length of returned results > 0 and handle that accordingly.
niftynei
force-pushed
the
nifty/bkpr_descriptions
branch
from
bad0b8c
to
cf26e66
Compare
| "resources": [ | ||
| "Main web site: <https://github.com/ElementsProject/lightning>" | ||
| ], | ||
| "examples": [ |
There was a problem hiding this comment.
did we want examples autogenerated for these?
There was a problem hiding this comment.
Yes! Do you know how to do that?
There was a problem hiding this comment.
Been trying since yesterday but it keeps erroring out, ill keep trying i need to debug more closely
plugins/bkpr/bookkeeper.c
Outdated
| && (lowest == 0 || lowest > chan->timestamp)) | ||
| lowest = chan->timestamp; | ||
| if (!param(cmd, buf, params, | ||
| p_req("identifier", param_outpoint, &outpoint), |
There was a problem hiding this comment.
needs to be 'outpoint' to match the schema or it cant find the param, ill update in my commit soon to come
niftynei
force-pushed
the
nifty/bkpr_descriptions
branch
2 times, most recently
from
ef8b66d
to
6ad7666
Compare
This takes an {payment_id} and {description}.
It looks for all chain + channel events that match
that {payment_id} and updates the description for those events.
We return all the updated events. If no events are updated, an empty
list is returned.
Changelog-Added: PLUGINS: bookkeeper has a new RPC `bkpr-editdescriptionbypaymentid` which will update the description for any event with matching payment_id
Given an {outpoint}, sets the description on the matching outpoint (if exists).
Note that if no outpoint exists in bookkeeper, will return an empty list
Changleog-Added: PLUGINS: bookkeeper has a new RPC `bkrp-editdescriptionbyoutpoint` which will set/update a description for an outpoint creation event.
Lets make sure that edit description works as intended.
niftynei
force-pushed
the
nifty/bkpr_descriptions
branch
from
6ad7666
to
d31e6c6
Compare
Make it such that you can update/set the description on bookkeeper events after they've been created.
Adds two new RPC commands to the bookkeeping plugin
bkpr-editdescriptionbypaymentidandbkpr-editdescriptionbyoutpointThese will update/set the description for any/all events that match the payment_id or outpoint, respectively. Returns the list of updated events, or an empty list if no events matched/were updated.
Note that for outpoints, only the deposit/creation event is updated with the description. "input" side events are left un-descripted.