Helloooooo frens! Welcome back! Let's get to building.

This is technically a continuation of day 16 but the work I did on day 16 was needed for this to work.

So the big things I need to do are:

- Complete the transition to the new transaction object storage type
- Make all the original tests work again

• Get ERC20 or ERC721 deposit and withdrawal (not resolution case withdrawal, just no-problem withdrawal) working

Basically I am going to make ERC20 and ERC721 work properly. Once that is done I can focus on cleaning it all up (and whether I can combine the functions somehow).

This is going to be a bit of work but since I'm making the following assumption, it should be easier:

• The user can send in ONE ERC20 token type or ONE individual ERC721 token per escrow transaction.

Now of course I can make a dynamically sized array and push as many of these new 'add-on' struct mappings to it if I want to. You know what that means?

IN THEORY (all caps for a good reason lol) you can send in 1000 DAI 1 WETH and 200 UNI as part of a single escrow transaction.

Does this benefit the user? Yes, gas is less.
Does this benefit us? Not really, but it's good for the user.

It would make frontend a little more complicated but I'm not up to that yet.

So let's get ERC721 deposit and withdrawal working and then do ERC20.

Since I've abstracted the logic into separate components, theoretically I only need one deposit and one withdrawal function. I'd prefer to keep the functions simple though, if possible.

Quick discussion on complex ERC721 resolution

Here is the planned process:

• The transaction is marked with a transactionState indicating it is awaiting payment
• A function redeemEscrow or similarly named exists which is either (a) payable and takes ETH or (b) token-based and takes WETH
• The function disperses funds sent in to both the party who is not receiving the NFT and to the DAO / contract (for fees)
• The NFT is sent out of the contract to the paying party

Either party can pay and the only difference is the percentage sent to the other party (since it depends who is paying).

This is how we can resolve partial NFT escrow disputes. The amount to pay in is set as fairMarketValue by the DAO when being resolved. If the floor price goes up then obviously it is in one party's interest to claim the token and if it goes down then it would stay locked there forever. We could have an updateFairMarketValue function that the DAO can call, but this would have to add to some FMV update counter which is used to incrementally increase the fee for the resolution, since this is extra work for the DAO to do and they should be compensated more.

Getting to work

Time to get to work. First thing we need to do is to make sure that the ERC721 deposit-allow-withdraw (from now on, I will call this DAW) cycle works. Non-DAW functions such as simple and complex resolutions are not being touched for now. Let's go:

There is no way for me to set the balance of this to 0. I don't know how I feel about the whole "set the balance to 0" thing anyway... Sure, it stops issues if people are exploiting reentrancy, but I think that the transactionState functions in the same way. I could also save on gas by removing it. I will leave it in for now.

No fees for ERC721 for now but if there is a transaction fee in the future, we will have to make payable withdrawals for it. I could just make the withdrawal payable tbh but that would require some form of NFT valuation model on-chain or something.

This is what it looks like at the moment. Ignore the typos. I changed it to safeTransferFrom eventually.

Appears to work and be 128k for a DAW. This is good. I like this.

If we are at 100k for ETH and 130k for a DAW (and obviously like 20k more for tokens) then I'm happy with that.

More struct changes?

I am not a big fan of transactions, to be honest I prefer baseTransactions or transactionBases etc. I don't really want to ruin the code just yet and do a full restructure like in last entry but I don't like that is is transactions...

Ignoring that for now, let's keep going

Our current ERC721 DAW costs are as follows:

I changed the initial tokenId to be 1 so that it isn't showing really low values for when the tokenId is 0.

Looks like it is about 120k for a DAW. Can't be certain yet but if it's 100k for a DAW + 20k per NFT that's a good price. You could move 3 or 4 BAYC NFTs for around the cost of a DEX swap (if it scaled).

Time to make sure ERC20 works

ERC20 costs 133k for a DAW.

Why is it more expensive?

We need to store the balance as a uint256 which means we are forced to use 2 slots. Unless the Wei balance was less than 2^96 - 1 (which is 7.9228162514264337593543950335 × 10^10) we have to do this. 10^10 is not enough to store things like SHIBA and other tokens with huge max supply values. We are forced to use uint256 if we want adoption unfortunately!

Current DAW costs

• 108k for ETH
• 133k for ERC20
• 120k for ERC721

These are good numbers. They aren't LlamaPay good (especially for ERC20) but the reality is that our use-case is different and requires 2 slots (40k gas for storage) minimum + 1 for the most basic use-case. That means we are using 25k gas for all other computation - including emit event stuff, some console2.log() which won't be in production and the transfer costs.

Base transaction object optimisations

We have two things to optimise:

1. expiryTime can be improved by copying how Gnosis does it (thanks to @AureliusBTC for this suggestion)
2. txPassthroughPercentage only needs to be 10,000 max so can take up less bits.

Let's handle #2 first:

txPassthroughPercentage is at maximum 10,000 (basis points) so can be uint16.

It is not uint16 and it is better that way - though since we want the default value as 0, we need to make sure that it is named properly. It is not percentagePassthrough or percentageToRecipient when the default is 0. The correct name should be percentRefund. Let's rename it!

I will add a to-do here to make sure that it's actually refunding this amount to the depositor rather than it being the passthrough percentage since it's quite convoluted...

Okay the variables are all called txPercentageRefund so now it is more readable and makes more sense.

Time to do #1:

Looks like epoch token locker was what Aurelius was referring to:

This locks tokens and uses some method to check the time and compare it against the current time.

Let's have a look:

![[17_9.png]]

Gnosis seems to store time in 5 minute blocks (BATCH_TIME) which means that changes occur every 5 minutes rather than every second.

Of course, I like the idea of this in theory but I don't want to limit people to perform actions in 5 minute blocks as that will limit usage of the protocol long-term.

block.timestamp is a uint256. Let's figure out what we need first.

Max uint32 is 4294967295
Current time is ~1650000000

This can store the timestamp for at least 50 more years. This should be fine as long as our contract is upgradeable. If we can spare another byte we can make it uint40 which will last practically forever.

Here is the revised structure:

This takes up 400 bits total. We can't make it 1 slot anyway since we need both addresses, so I will use uint64 for expiryTime since it is a more commonly used format and it won't affect anything. Now we are at 424 bits. We could theoretically store the fee being charged in here.

I can also add a 'spent' boolean here as a form of reentrancy protection if I want.

A gas optimisation I just thought of is to write to the struct all at once when we are doing withdrawals. We can clone the object first and then edit the fields and then write it all at the end.

Quick detour - implementing that SSTORE optimisation

Since the EVM works in 32 byte chunks, it is best to write to the baseTransactionObject struct just once as opposed to twice (but only if the two SSTORES were in the same 32 byte chunk of the struct).

Let's see if we can optimise our withdrawal functions by doing that...

Our only two SLOAD interactions for withdrawEth are a change to the balance value (which is in its own 32 byte chunk) and to the txState enum which is in another chunk.

Using this knowledge, it is theoretically possible to add a second write to another variable called isClaimed in the baseTransactionObject struct and at the cost of one more SLOAD, have good reentrancy protection (so long as we follow CEI and update the struct before we do any interactions). I almost feel like doing this to be honest, just so I can remove OZ's mutex (it costs 3000 gas and this proposed method costs nothing extra).

Renaming functions

I don't like how it is depositEth and not depositETH so I will rename things.

Done!

Removing the old OpenZeppelin mutex / nonReentrant modifier

With it:

Without it:

It saves about 500 gas for ETH but almost 2500 for ERC20.

This is a pretty big saving if I can do it.

Let's commit and push up changes first.

Removing OZ mutex and using our own implementation?

The theory behind this is that if we can make use of our SSTORE and SLOAD to the main baseTransactionObject in storage, we can effectively have our own mutex. There is one small caveat - it makes transactions single-spend (so we can't have ERC20 / ERC721 composite transactions).

As much as I like this idea for the 2500 gas savings, I want to make sure that I'm not making a dumb decision and limiting what i can build.

Delaying that - Multiple token input

Let's see if I can create some form of implementation which allows multiple ERC20 tokens as inputs.

Let's record the gas at the start for the current (single token input) function:

Now let's start! Let's write up the test first:

This is the before of the function definition:

I am having issues getting the dynamically sized array to work.

My approach to editing my code was wrong - I should have duplicated the function instead of editing the one that was there. I will undo it and go again...

Abandoned changes and am doing the function again but this time it will be a new function.

Working with dynamically sized arrays is turning out to be a bit annoying...

I am currently working with dynamically sized arrays for testing of the function.

The goal is that:

1. Zero token addresses or amounts will revert
2. If the number of addresses and amounts are not the same, it will revert
3. One token address and amount will cost the same (+/- 500 gas) as the old function
4. Multiple addresses and amounts will be able to be added and scale costs by 40k gas per token (storage cost only).

Assuming that the code is optimised, this will be something I can implement in withdraw (in some weird CEI way, which I will figure out since otherwise I'm sure there are some fancy ways to attack it) and have a flexible number of ERC20 tokens be able to be sent in a single transaction. I can of course impose a limit to prevent attacks which exploit overflows / hitting the gas limit etc. I think 5-10 tokens at most is a good amount.

If this works I will extend it to ERC721 and therefore will be able to have:

• ETH
• ERC20 one or more (multiple tokens)
• ERC721 one or more (multiple collections)

Which would make this the most powerful escrow service available.

However the security aspect of using dynamically sized arrays is pretty hard for me to estimate at this stage. That is all for today!

See you tomorrow!