-
Notifications
You must be signed in to change notification settings - Fork 19
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.
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
Util Contract #470
base: main
Are you sure you want to change the base?
Util Contract #470
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some debug messages e.g. console.log
and comment out lines
please remove them if they is no need for them
packages/contracts/contracts/attester-utils/ReimburseAttestation.sol
Outdated
Show resolved
Hide resolved
packages/contracts/contracts/attester-utils/ReimburseAttestation.sol
Outdated
Show resolved
Hide resolved
packages/contracts/contracts/attester-utils/ReimburseAttestation.sol
Outdated
Show resolved
Hide resolved
packages/contracts/contracts/attester-utils/ReimburseAttestation.sol
Outdated
Show resolved
Hide resolved
packages/contracts/contracts/attester-utils/ReimburseAttestation.sol
Outdated
Show resolved
Hide resolved
packages/contracts/contracts/attester-utils/ReimburseAttestation.sol
Outdated
Show resolved
Hide resolved
packages/contracts/contracts/attester-utils/ReimburseAttestation.sol
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
structure looks good, tests can be simplified
packages/contracts/contracts/attester-utils/ReimburseAttestation.sol
Outdated
Show resolved
Hide resolved
packages/contracts/contracts/attester-utils/ReimburseAttestation.sol
Outdated
Show resolved
Hide resolved
packages/contracts/contracts/attester-utils/ReimburseAttestation.sol
Outdated
Show resolved
Hide resolved
packages/contracts/contracts/attester-utils/ReimburseAttestation.sol
Outdated
Show resolved
Hide resolved
packages/contracts/contracts/attester-utils/ReimburseAttestation.sol
Outdated
Show resolved
Hide resolved
packages/contracts/contracts/attester-utils/ReimburseAttestation.sol
Outdated
Show resolved
Hide resolved
packages/contracts/contracts/attester-utils/ReimburseAttestation.sol
Outdated
Show resolved
Hide resolved
packages/contracts/contracts/attester-utils/ReimburseAttestation.sol
Outdated
Show resolved
Hide resolved
packages/contracts/contracts/attester-utils/ReimburseAttestation.sol
Outdated
Show resolved
Hide resolved
packages/contracts/contracts/attester-utils/ReimburseAttestation.sol
Outdated
Show resolved
Hide resolved
packages/contracts/contracts/attester-utils/ReimburseAttestation.sol
Outdated
Show resolved
Hide resolved
packages/contracts/contracts/attester-utils/ReimburseAttestation.sol
Outdated
Show resolved
Hide resolved
) public payable virtual { | ||
require(whitelist[msg.sender], 'This user is not in the whitelist'); | ||
|
||
uint256 attesterSignalIndex = unirep.numEpochKeyNoncePerEpoch(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the signal index is not numEpochKeyNoncePerEpoch
just hardcode like uint256 attesterSignalIndex = 2;
uint8 public immutable numEpochKeyNoncePerEpoch; | ||
|
||
constructor() { | ||
numEpochKeyNoncePerEpoch = 2; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is not used here
Pull request checklist
Please check if your PR fulfills the following requirements:
yarn build
) was run locally in root directory and any changes were pushedyarn lint:fix
) was run locally in root directory and any changes were pushedPull request type
Please check the type of change your PR introduces:
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this introduce a breaking change?
Other information