- 
                Notifications
    You must be signed in to change notification settings 
- Fork 75
feat: add a first draft of the proof library #8
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
Conversation
Signed-off-by: Matt Rice <matthewcrice32@gmail.com>
| // This array designates each address that must be refunded. | ||
| address[] refundAddresses; | ||
| // This array designates how much each of those addresses should be refunded. | ||
| uint256[] refundAmounts; | 
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.
Does the ordering of this struct matter in terms of packing types together?
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.
I don't think so:
- The only type that's currently <32 bytes is address, which is in an array.
- Array elements are padded to 32 bytes even if using abi.encodePacked.
        
          
                contracts/MerkleLib.sol
              
                Outdated
          
        
      | // TODO: some of these data structures can be moved out if convenient. | ||
| // This data structure is used in the settlement process on L1. | ||
| // Each PoolRepayment structure is responsible for balancing a single chain's SpokePool across all tokens. | ||
| struct PoolRepayment { | 
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.
nit: repayment sounds like paying back the relayer. Why not use another more clear word like "rebalance"?
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.
Good point. Will update.
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.
Fixed.
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.
Good start +1.
        
          
                contracts/MerkleLib.sol
              
                Outdated
          
        
      | // HubPool. There can be arbitrarily complex rebalancing rules defined offchain. This number is only nonzero | ||
| // when the rules indicate that a rebalancing action should occur. This means that the DestinationDistribution | ||
| // associated with this PoolRebalance can indicate a balance change, but that the HubPool does not rebalance. | ||
| // When a rebalance does occur, the netSendAmount is the sum of all netBalanceChange values in the | 
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.
this does not match the variables in the DestinationDistribution. you refer to netBalanceChange in the comment but there is no such variable.
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.
Agreed. Will fix these up.
| // Used to verify that this is being decoded on the correct chainId. | ||
| uint256 chainId; | ||
| // This is the amount to return to the HubPool. This occurs when there is a PoolRebalance netSendAmount that is | ||
| // negative. This is just that value inverted. | 
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.
I'm not sure if we want the inverted amount. it might be easier if this is always a int256. a positive number always means L1->L2 flow and a negative number always means L2->L1 flow. dont bake any polarity changes into the variable.
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.
As discussed today, we would only set this number if we wanted to tell the receiving chain to send tokens back to mainnet. So I made this positive since there is no need to communicate when tokens are being sent to the L2 (aka the opposite sign).
| } | ||
|  | ||
| // This leaf is meant to be decoded in the SpokePool in order to pay out individual relayers for this bundle. | ||
| struct DestinationDistribution { | 
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 was a bool within this struct at one point to indicate if amountToReturn should be sent. what happened to this?
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.
As discussed today, this is no longer needed. We don't need to communicate when the rebalances are happening in this data structure. The netSendAmount in the struct above is the only place where we denote whether money is being sent to the chain or not.
Co-authored-by: Chris Maree <christopher.maree@gmail.com>
* chore: formatting fix (#1) Signed-off-by: Matt Rice <matthewcrice32@gmail.com> * Arbitrum, Ink, Mode, ZkSync * chore: Deploy Blast SpokePool implementation (#3) * chore: Deploy AlephZero SpokePool implementation (#6) * chore: Deploy WorldChain SpokePool implementation (#5) * deploy base lisk eth redstone scroll soneium spoke implementations (#4) Signed-off-by: Matt Rice <matthewcrice32@gmail.com> Signed-off-by: bennett <bennett@umaproject.org> Co-authored-by: Matt Rice <matthewcrice32@gmail.com> * chore: add deployments for linea, optimism, polygon, and zora (#7) Signed-off-by: Matt Rice <matthewcrice32@gmail.com> * fix(test): Require transfer to msg.sender on fill (#8) * fix(artifacts): update artifacts with implementation address Signed-off-by: bennett <bennett@umaproject.org> --------- Signed-off-by: Matt Rice <matthewcrice32@gmail.com> Signed-off-by: bennett <bennett@umaproject.org> Co-authored-by: nicholaspai <npai.nyc@gmail.com> Co-authored-by: Paul <108695806+pxrl@users.noreply.github.com> Co-authored-by: bmzig <57361391+bmzig@users.noreply.github.com> Co-authored-by: bennett <bennett@umaproject.org>
Note: the proof verification functions all return true for now to make interacting with them simpler until we have some js or ts that can be used to construct them.