Privacy e2e #32

Mikelle · 2024-04-24T20:12:14Z

This PR implements privacy e2e protocol.
Related docs:

contracts/contracts/Oracle.sol

p2p/pkg/contracts/bidder_registry/bidder_registry.go

p2p/pkg/contracts/block_tracker/block_tracker.go

aloknerurkar · 2024-04-26T13:34:46Z

p2p/pkg/depositmanager/deposit.go

@@ -0,0 +1,189 @@
+package depositmanager


No unit tests for this pkg. We should add some basic tests atleast.

p2p/pkg/depositmanager/deposit.go

aloknerurkar · 2024-04-26T13:46:41Z

p2p/pkg/depositmanager/deposit.go

+	eg.Go(func() error {
+		ev1 := events.NewEventHandler(
+			"NewWindow",
+			func(window *blocktracker.BlocktrackerNewWindow) error {


There should be some way to clear old window information, otherwise the state will keep growing and its all in memory now.

I'm not sure that I understand this comment. How will the state grow if I'm using atomicInt here?

I meant for the balances. So we will have old window information everywhere. We should have some way to clean that up. Maybe LRU cache with fixed sizes could be good. Not an immediate concern.

p2p/pkg/events/events.go

p2p/pkg/preconfirmation/preconfirmation.go

contracts/contracts/BlockTracker.sol

ckartik · 2024-04-28T20:18:10Z

contracts/contracts/BlockTracker.sol

+    }
+
+    // Function to get the winner of a specific block
+    function getBlockWinner(


This is a cool idea!

ckartik · 2024-04-28T20:24:01Z

p2p/pkg/contracts/preconf/preconf.go

 	bidSignature []byte,
 	commitmentSignature []byte,
-) error {
+	sharedSecretKey []byte,


Just to understand the flow, where does this sharedSecretKey originate?

In code it's here:
https://github.com/primevprotocol/monorepo/pull/32/files#diff-a9be9ab22daa2b86b99b82b397e5590bbc5d53f0a81f366b88bbc557aeab16ffR119

In docs here is a good image, describing the encryption process:
https://www.notion.so/primev/mev-commit-privacy-e2e-v1-1-6569036f5f50438885df24050a54ff17?pvs=4#f1a1eec07bd54c3098090e804d1316c7

Mikelle · 2024-04-29T14:55:40Z

contracts/contracts/BlockTracker.sol

+    uint256 public blocksPerWindow = 64;
+
+    // Mapping from block number to the winner's address
+    mapping(uint256 => address) public blockWinners;


@aloknerurkar I think it would be great to add some state pruning function, bcs otherwise state will grow indefinitely and record tx will be super expensive

p2p/pkg/depositmanager/deposit.go

aloknerurkar · 2024-04-29T18:01:10Z

p2p/pkg/keyexchange/keyexchange.go

+	return providers, nil
+}
+
+func (ke *KeyExchange) prepareMessages(providers []p2p.Peer) ([][]byte, []byte, error) {


Sorry I missed this in my previous reviews. Not sure why we are sending all then encrypted keys to all the peers?

We should have point to point messages. For eg. each provider should be sent the key meant for it only, along with the timestamp message. If a new provider joins the network, only that provider should be getting a new message with its encrypted key.

We should make these changes before the release as this protobuf message structure takes in the array of keys. If we release with this then changing the protobuf would involve protocol bumps.

I'm taking it from the design document...

p2p/pkg/p2p/libp2p/internal/handshake/handshake.go

aloknerurkar

There are some improvements that still come to mind. But I think we can merge this PR as its become too big. Then the rest of the things can be done in separate PRs.

Just make sure the integration tests run and then merge the changes.

ckartik · 2024-05-03T19:12:57Z

p2p/pkg/contracts/preconf/preconf.go

+		decayEndTimeStamp int64,
+		bidSignature []byte,
+		commitmentSignature []byte,
+		sharedSecretKey []byte,


we might want to call this a shared blinder, but up to you. SharedSecretKey gives the implicit assumption it's being used for encryption. Blinder also helps make it apparent that it's ephemeral.

But feel free to change the name, after and merge this PR, this is def not a blocker comment!

ckartik · 2024-05-03T19:24:54Z

p2p/pkg/preconfirmation/preconfirmation.go

-
-	ethAddress, err := p.signer.VerifyBid(bid)
+	p.logger.Info("received bid", "encryptedBid", encryptedBid)
+	bid, err := p.encryptor.DecryptBidData(peer.EthAddress, encryptedBid)


What happens if the encryptedBid came from a Peer that didn't have peer.EthAddress. Maybe we should add a NOTE or TODO here to update this section if we ever move to a gossip protocol, We'll need the decryptor to attempt decryption from all possible keys.

The ecnryptedBid payload should also have a bidder signature on it right? maybe use that to verify identity?

aloknerurkar · 2024-05-03T19:24:13Z

p2p/pkg/preconfirmation/preconfirmation.go

 type BidProcessor interface {
 	ProcessBid(context.Context, *preconfpb.Bid) (chan providerapi.ProcessedBidResponse, error)
 }

+type EncrDecrCommitmentStore interface {


This should be removed. Not required.

aloknerurkar · 2024-05-03T19:26:54Z

p2p/pkg/depositmanager/deposit.go

+					return err
+				}
+
+				effectiveStake := new(big.Int).Div(new(big.Int).Set(bidderReg.DepositedAmount), new(big.Int).SetUint64(blocksPerWindow))


nit: do we need to do new(big.Int).Set here or cant we just use the DepositedAmount directly. The result is anyway a new big.Int

aloknerurkar · 2024-05-03T19:27:42Z

p2p/pkg/depositmanager/deposit.go

+	return dm.store.RefundBalanceForBlock(address, deductedAmount, blockNumber)
+}
+
+func (dm *DepositManager) checkAndSetBlocksPerWindow() (uint64, error) {


Instead of checkAndSet, we should name it get or getOrSet as this returns the value.

Mikelle requested a review from aloknerurkar April 24, 2024 20:12

Mikelle self-assigned this Apr 24, 2024

Mikelle force-pushed the privacy-e2e branch 2 times, most recently from 7ace486 to 456b835 Compare April 25, 2024 19:57

aloknerurkar mentioned this pull request Apr 26, 2024

feat(oracle): e2e privacy changes #35

Merged

aloknerurkar reviewed Apr 26, 2024

View reviewed changes

aloknerurkar reviewed Apr 27, 2024

View reviewed changes

contracts/contracts/BlockTracker.sol Outdated Show resolved Hide resolved

ckartik reviewed Apr 28, 2024

View reviewed changes

Mikelle commented Apr 29, 2024

View reviewed changes

aloknerurkar reviewed Apr 29, 2024

View reviewed changes

Mikelle force-pushed the privacy-e2e branch from 790e46f to c6e736b Compare April 30, 2024 18:02

aloknerurkar approved these changes Apr 30, 2024

View reviewed changes

Mikelle force-pushed the privacy-e2e branch from be24bf8 to 54bc581 Compare May 1, 2024 16:19

shaspitz mentioned this pull request May 1, 2024

feat: upgradable contracts #40

Closed

Mikelle force-pushed the privacy-e2e branch 5 times, most recently from 3391e62 to f1cbbed Compare May 3, 2024 14:34

ckartik reviewed May 3, 2024

View reviewed changes

aloknerurkar reviewed May 3, 2024

View reviewed changes

Mikelle force-pushed the privacy-e2e branch from dcb7a6f to 81b4ba2 Compare May 3, 2024 23:27

Mikelle and others added 6 commits May 9, 2024 11:42

feat: upload contract changes

663468f

feat: updated abi

bec469b

feat: upload p2p (mev-commit) changes for privacy e2e

39d5948

feat(oracle): e2e privacy changes (#35)

71e618b

refactor: moved to x packages

f5b565a

refactor: get rid of logs parsing, redundant checks and function in sc

a6ce840

Mikelle and others added 27 commits May 9, 2024 11:42

fix: move blocksPerWindow set up to separated func

f10abd6

fix: updated artifacts workflow

8d14c79

fix: added missing goreleaser to real bidder

0a47426

fix: updated dir in goreleaser

da322b0

feat: added logs to updater

6b0abcb

refactor: changed 10000 to timeDiff in real-bidder

87c5e6b

feat(tracker): split preconf protocol for better organisation (#44)

078cf3c

fix: round decay to 100 if greater

7d79ea4

fix: updated abis and protos

bd00712

fix: oracle changes after decay merge

452c004

fix: p2p after merge

82d4ea7

chore: increased dispatch window

5d1f9f2

chore: added wait receipt for testing

dcc2295

chore: deleted wait receipt

df2f559

chore: added logs for debug

d945a69

chore: added log for encrypted commitment stored

d55fd16

fix: sync in store

d017ef7

chore: pr clean ups

1b604af

fix: deleted redundant method

a499e26

chore: updated logs

3fe4ae7

chore: deleted curly brackets from storeEncr func

5fcb4b0

chore: increased dispatchWindow for test

86ee5c2

chore: clean up redundant logs

f092498

chore: fixed SCs warnings

9afeb58

chore: fixed growing state in encryptor

fac9362

chore: changed min time for the real bidder

041573e

fix: deep copy txnHashes in test

c24fe2b

Mikelle force-pushed the privacy-e2e branch from e0ea1ee to c24fe2b Compare May 9, 2024 08:42

Mikelle merged commit d59f069 into main May 9, 2024
5 checks passed

Mikelle deleted the privacy-e2e branch May 9, 2024 08:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Privacy e2e #32

Privacy e2e #32

Mikelle commented Apr 24, 2024

aloknerurkar Apr 26, 2024

aloknerurkar Apr 26, 2024

Mikelle Apr 29, 2024

aloknerurkar Apr 29, 2024

ckartik Apr 28, 2024

ckartik Apr 28, 2024

Mikelle Apr 29, 2024

Mikelle Apr 29, 2024

aloknerurkar Apr 29, 2024

Mikelle Apr 29, 2024

aloknerurkar left a comment

ckartik May 3, 2024

ckartik May 3, 2024

ckartik May 3, 2024

ckartik May 3, 2024

aloknerurkar May 3, 2024

Mikelle May 3, 2024

aloknerurkar May 3, 2024

Mikelle May 3, 2024

aloknerurkar May 3, 2024

Mikelle May 3, 2024

Privacy e2e #32

Privacy e2e #32

Conversation

Mikelle commented Apr 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aloknerurkar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment