Improvements needed on the event-based features #84
Description
High
-
The decryption key endpoint has not been updated, and does not return decryption keys for event-based identities. (High) (for now return all decryption keys for identity, later allow for identity per eon)==> Implement GetDecryptionKey endpoint for event based decryption triggers #86 -
The decrypt commitment should also be adjusted (High)==> Implement GetDecryptionKey endpoint for event based decryption triggers #86 -
/get_event_decryption_key throws a 500 error due to a nil-pointer exception (High)==> /get_event_decryption_key throws a 500 error due to a nil-pointer exception #88 -
Add unit tests that cover the new endpoints (High)
-
Indexed arg filters ignore ABI position; non‑first indexed args map to wrong topic offsets (High) Indexed arg filters ignore ABI position; non‑first indexed args map to wrong topic offsets #90
-
Manual padding needed for non‑indexed, non‑uint256 args: left‑pad bytes to 32 bytes (High)
-
Swagger docs not generated correctly - see compile event trigger endpoint (High)
Medium
-
Check whether dynamic types (string, bytes, arrays) are supported for non‑indexed arguments: https://github.com/shutter-network/shutter-api/blob/main/internal/usecase/eventtrigger.go#L221 (Dynamic types are considered like static types here starting at a certain offset with a fixed length when their encoding is different, the head contains an offset to where the length and actual content is) (Medium) - if not supported, document for now
-
Update the contract / potentially rolling shutter to check for duplicates:
-
allow same identity registration as long as the expiration block is higher than the previous one (only API since only API using contract)
-
if a user submits a registration with different event triggers, the second would take precedence over the previous registration -> Change primary key to eon, identity here: https://github.com/shutter-network/rolling-shutter/blob/main/rolling-shutter/keyperimpl/shutterservice/database/sql/migrations/V2_event_based_triggers.sql#L13C18-L13C46 as well as the update on conflict query, also in fired triggers
-
what if we want to allow different ttls with the same identity
See comment here: Add endpoints for event based decryption triggers #79 (comment) (Medium) -
About the ttl / expiration block, we would be returning 0 while we wait for the tx to be mined, could be updated to pending perhaps but 0 could also mean the same as pending
See comment here: Add endpoints for event based decryption triggers #79 (comment) (Low) -
Add a documentation note to let the user know that they need to check for the transaction hash success to ensure the registration has succeeded / check endpoint behavior when transaction does not succeed (Medium)
-
Design improvement: Revisit GetDataForEncryption endpoint #83
-
Add a prefix for both time-based and event-based endpoints that would redirect to the same backend endpoint if needed (e.g. /time/register_identity versus /event/register_identity) - check if there is a version in the path (Medium) Add a prefix for both time-based and event-based endpoints #92
-
Fix tests
TestShutterServiceSuite/TestDecryptionCommitment,TestShutterServiceSuite/TestDecryptionCommitmentNotFound,TestShutterServiceSuite/TestGetDataForEncryptionInvalidSender(Medium) Fix tests and add new tests for event based #96 -
Check the integration test suite still works / adapt it for the new features (Medium)
Low
-
The API can still be deployed even if the event registry contract isn’t deployed; event endpoints will just fail at runtime with a 500 error. Nice‑to‑have: add a feature flag to enable/disable event triggers while still serving endpoints. If the contract isn’t deployed, return a 503 Service Unavailable error. Not required for the upcoming deployment. (Low)
-
Add intended use for /get_data_for_encryption (Low)
-
Update Swagger docs description to include event-based triggers as well (Low)