Skip to content

Delayed transactions v1#68

Open
evelinemolnar wants to merge 63 commits intofeat/v4from
delayed-transactions-v1
Open

Delayed transactions v1#68
evelinemolnar wants to merge 63 commits intofeat/v4from
delayed-transactions-v1

Conversation

@evelinemolnar
Copy link

No description provided.

cosmatudor and others added 30 commits August 21, 2024 14:25
…nagement

CII-56: Pending transactions management
@evelinemolnar evelinemolnar marked this pull request as ready for review September 19, 2024 09:13
}

if (singleTxThreshold == 0) {
singleTransactionThreshold[token] = defaultSingleTransactionThreshold;
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: do you think it will be better that, instead of using default values here, we should error as to prevent as much as we can bad values?
Valid also for aggregateThreshold


_processDelayedTransactions(tokenAddress);

uint256 currentBlock = block.number;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think it will be better to work with block.timestamp instead of block.number, or, at least keep also the timestamp when the deposit was made just to simplify the logic of time computing when we decide we can execute a transaction?

_addDelayedTransaction(tokenAddress, amount, recipientAddress, true, callData);
emit TransactionDelayed(msg.sender, tokenAddress, amount, recipientAddress, true);
return (0, 0);
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

useless else? :)

aggregatedValue[bucketId][tokenAddress] += amount;
(batchNonce, depositNonce) = _processDeposit(tokenAddress, amount, recipientAddress, callData);
return (batchNonce, depositNonce);
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

useless else?


function _processDelayedTransactions(address tokenAddress) internal {
uint256 i = 0;
while (i < delayedTransactions.length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this will consume a lot of gas. And the user y will actually pay for executing the deposit operations for user x, and so on. Thinking of a way to improve this and do the computation only when trying to assemble the current batch (I do not have a good idea now)

This method also has the disadvantage that, if a transaction is placed in a bucket, it won't be executed until a new deposit will be made. Which, if it will happen in 3 days, that will be the moment when it will actually be placed in a batch.

await safe.deposit(genericERC20.address, 400, recipientAddress);
await safe.deposit(genericERC20.address, 200, recipientAddress); // Delayed

// Simulate 24 hours (14,400 blocks at 6s per block)
Copy link
Contributor

Choose a reason for hiding this comment

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

6s or 12s? :)

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.

4 participants