Skip to content
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

feat(IntentSource): vault based intents #128

Open
wants to merge 9 commits into
base: AUDIT
Choose a base branch
from

Conversation

re1ro
Copy link
Collaborator

@re1ro re1ro commented Dec 16, 2024

  • IntentSource: storage-less intents

@re1ro re1ro self-assigned this Dec 16, 2024
Copy link
Contributor

@carlosfebres carlosfebres left a comment

Choose a reason for hiding this comment

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

It's looking great! I left a couple of questions

Comment on lines 62 to 64
function intentVaultAddress(
Intent calldata intent
) internal view returns (address) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can the intentVaultAddress function be a pure function since it's not reading any state or is it required to access calldata?

Comment on lines 176 to 189
function withdrawRewards(Intent calldata intent) public {
bytes32 intentHash = keccak256(abi.encode(intent));
address claimant = SimpleProver(intent.prover).provenIntents(
intentHash
);

if (claimant != address(0) && !claimed[intentHash]) {
vaultClaimant = claimant;
claimed[intentHash] = true;

emit Withdrawal(intentHash, claimant);
} else {
if (block.timestamp < intent.expiryTime) {
revert UnauthorizedWithdrawal(intentHash);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we mark the intent as claimed when the intent has expired and the creator claims it back?

@nknavkal
Copy link
Collaborator

can this PR be against main rather than AUDIT? just dont want to mix those changes up order-wise

re1ro added 4 commits January 3, 2025 23:06
# Conflicts:
#	contracts/Inbox.sol
#	contracts/IntentSource.sol
#	contracts/interfaces/IInbox.sol
#	contracts/interfaces/IIntentSource.sol
#	contracts/types/Intent.sol
#	yarn.lock
@re1ro re1ro force-pushed the feat/vault-based-intents branch from 559341f to f01475e Compare January 7, 2025 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants