Idempotent apis

[peterbroadhurst]

This feature allows API requests to FireFly Core to be made idempotent.

That is that the caller of the API can specify its own unique identifier (an arbitrary string up to 256 characters) that uniquely identifies the request. So if there is a network connectivity failure, or an abrupt termination of either runtime, the application can safely attempt to resubmit the REST API call and be returned a 409 Conflict HTTP code.

Examples of how an app might construct such an idempotencyKey include:

• Unique business identifiers from the request that comes into its API up-stream - passing idempotency along the chain
• A hash of the business unique data that relates to the request - maybe all the input data of a blockchain transaction for example, if that payload is guaranteed to be unique.

  Be careful of cases where the business data might not be unique - like a transfer of 10 coins from A to B... that
  could happen multiple times, and each would be a separate business transaction. Where as transfer with invoice
  number abcd1234 of 10 coins from A to B might be perfectly unique.

• A unique identifier of a business transaction generated within the application and stored in its database before submission

  Here be careful you haven't just moved the problem up one layer. How does that unique ID get generated? Is that
  itself idempotent?

FireFly already uses an idempotent interface downstream to key plugins:

• EVMConnect blockchain operations - fully idempotent, using the operation ID
• Token operations - inherit idempotence from the blockchain connector they communicate with

Note that the interface between FireFly and EthConnect for blockchain operations is not currently fully idempotent.
This PR does not make the changes needed for that, and I note that EVMConnect is the recommended choice for
new FireFly projects.

Tasks

• Add idempotencyKey field to messages
  • Persisted directly on Message object
  • Local only - not propagated in persistBatch when received remotely
  • Note complexity was in the batching logic, as it's important if the message writer batching has one out of a set of messages that fails the idempotency check, it doesn't fail the whole batch of message/data inserts. To do this without reducing performance for the good case took a bit of extra work
• Add idempotencyKey to invoke of blockchain smart contracts
  • Persisted on Transaction object
• Add idempotencyKey to deploy of blockchain smart contracts
  • Persisted on Transaction object
• Add idempotencyKey to transfer (/mint/burn/approval) of tokens
  • Persisted on Transaction object
• Add idempotencyKey to publish APIs
• Update e2e tests for Message to test idempotent submission
• Update e2e tests for Tokens to test idempotent submission
• Update e2e tests for Custom contracts to test idempotent submission

I did look at in this PR, whether I could pass the idempotencyKey back to applications on events if they submitted
them on a request. However, this proved to need more investigation because the .transaction field of events is
only enriched on transaction_submitted events. To switch it to be cache-back-queried on every event, and to bloat
the JSON with the full transaction for all events, felt like a change that needed discussion independent from adding
the idempotencyKey capability to the submission APIs.

[codecov-commenter]

Codecov Report

Merging #1096 (3ffb370) into main (0e6ae46) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 3ffb370 differs from pull request most recent head deddc41. Consider uploading reports for the commit deddc41 to get more accurate results

@@           Coverage Diff           @@
##             main    #1096   +/-   ##
=======================================
  Coverage   99.98%   99.98%           
=======================================
  Files         308      309    +1     
  Lines       20467    20553   +86     
=======================================
+ Hits        20463    20549   +86     
  Misses          4        4           

Impacted Files	Coverage Δ
internal/assets/manager.go	100.00% <ø> (ø)
pkg/core/contracts.go	100.00% <ø> (ø)
pkg/core/message.go	100.00% <ø> (ø)
pkg/core/tokenpool.go	100.00% <ø> (ø)
pkg/core/transaction.go	100.00% <ø> (ø)
internal/apiserver/route_post_data_blob_publish.go	100.00% <100.00%> (ø)
...nternal/apiserver/route_post_data_value_publish.go	100.00% <100.00%> (ø)
internal/apiserver/route_post_token_pool.go	100.00% <100.00%> (ø)
internal/assets/token_approval.go	100.00% <100.00%> (ø)
internal/assets/token_pool.go	100.00% <100.00%> (ø)
... and 11 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[awrichar]

No idempotency on this one?

[awrichar]

-1?

[awrichar]

Is there an edge case where two messages in the same batch could reference the same piece of data, and only one gets removed?

[awrichar]

Looks very good. One edge case noted for consideration.