Skip to content

refactor(gateway-contracts): remove redundant decryptionDone storage for public decryption #1532

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
enitrat wants to merge 2 commits into
base: main
Choose a base branch
from
Open

refactor(gateway-contracts): remove redundant decryptionDone storage for public decryption #1532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cafea4b
refactor(gateway-contracts): remove redundant decryptionDone storage …
enitrat Dec 10, 2025
5846288
update bindings
enitrat Dec 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions gateway-contracts/contracts/Decryption.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,8 @@ contract Decryption is
// ----------------------------------------------------------------------------------------------
// Common decryption state variables:
// ----------------------------------------------------------------------------------------------
/// @notice Whether a (public, user, delegated user) decryption is done
mapping(uint256 decryptionId => bool decryptionDone) decryptionDone;
/// @notice Whether a (user, delegated user) decryption is done
mapping(uint256 decryptionId => bool userDecryptionDone) userDecryptionDone;
Copy link
Contributor

@isaacdecoded isaacdecoded Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that, unfortunately, we can't rename state variables, but deprecate them and add the new ones at the end of the storage struct. This is because of upgrade conflicts between storage layouts. Also, we must bump the contract and the reinitializer versions in order to properly upgrade the contract once the change is introduced.

Copy link
Contributor Author

@enitrat enitrat Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ok cool thanks for raising that!

Copy link
Contributor Author

@enitrat enitrat Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, we must bump the contract and the reinitializer versions in order to properly upgrade the contract once the change is introduced.

what does this mean?

Copy link
Contributor

@isaacdecoded isaacdecoded Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means:

  1. Bump internal contract version here
  2. And also bump the reinitializer version here

The second one is relevant for upgrading the contract, so the reinitializerVX method can be called with the correct versioning. For example, on testnet we have deployed now v0.10.x with reinitializer version 4, so when we need to upgrade to v0.11.x we should have this reinitializer version incremented to 5 in order to properly call the reinitializerV4() method.

// prettier-ignore
/// @notice Whether KMS signer has already responded to a decryption request.
mapping(uint256 decryptionId =>
Expand Down Expand Up @@ -406,9 +406,7 @@ contract Decryption is

// Send the event if and only if the consensus is reached in the current response call.
// This means a "late" response will not be reverted, just ignored and no event will be emitted
if (!$.decryptionDone[decryptionId] && _isConsensusReachedPublic(verifiedSignatures.length)) {
$.decryptionDone[decryptionId] = true;

if ($.decryptionConsensusDigest[decryptionId] == bytes32(0) && _isConsensusReachedPublic(verifiedSignatures.length)) {
// A "late" valid KMS could still see its transaction sender address be added to the list
// after consensus. This storage variable is here to be able to retrieve this list later
// by only knowing the decryption ID, since a consensus can only happen once per decryption
Expand Down Expand Up @@ -671,8 +669,8 @@ contract Decryption is

// Send the event if and only if the consensus is reached in the current response call.
// This means a "late" response will not be reverted, just ignored and no event will be emitted
if (!$.decryptionDone[decryptionId] && _isThresholdReachedUser(txSenderAddresses.length)) {
$.decryptionDone[decryptionId] = true;
if (!$.userDecryptionDone[decryptionId] && _isThresholdReachedUser(txSenderAddresses.length)) {
$.userDecryptionDone[decryptionId] = true;

// Since we use the default value for `bytes32`, this means we do not need to store the
// digest in `decryptionConsensusDigest` here like we do for the public decryption case.
Expand Down Expand Up @@ -783,7 +781,11 @@ contract Decryption is
*/
function isDecryptionDone(uint256 decryptionId) external view virtual returns (bool) {
DecryptionStorage storage $ = _getDecryptionStorage();
return $.decryptionDone[decryptionId];
// Public decryption: check if consensus digest has been set
if (decryptionId > PUBLIC_DECRYPT_COUNTER_BASE && decryptionId <= USER_DECRYPT_COUNTER_BASE) {
return $.decryptionConsensusDigest[decryptionId] != bytes32(0);
}
return $.userDecryptionDone[decryptionId];
}

/**
Expand Down
8 changes: 4 additions & 4 deletions gateway-contracts/rust_bindings/src/decryption.rs
View file
Open in desktop

Large diffs are not rendered by default.

Loading