Skip to content

Commit

Permalink
Extract block height from proof (#67)
Browse files Browse the repository at this point in the history 
* draft getBlockHeightFromProof

* debug

* fix extract block height from proof

* fix tests and contract

* separate BlockHeightFromProofExtractor

* add new test

* fix tests

* fix tests

* add checkPreMigration flag

* add test with switched off preMigration check

* add comments about proof structure

* add comment for withdraw function

* checkPreMigration -> receiptBlockHeight
  • Loading branch information
@olga24912
olga24912 authored Aug 3, 2024
1 parent 3f3f420 commit 02d3f22
Show file tree
Hide file tree
Showing 3 changed files with 180 additions and 26 deletions.
81 changes: 81 additions & 0 deletions eth-custodian/contracts/BlockHeightFromProofExtractor.sol
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity ^0.8;

import "rainbow-bridge-sol/nearbridge/contracts/Borsh.sol";

library BlockHeightFromProofExtractor {
using Borsh for Borsh.Data;
using BlockHeightFromProofExtractor for Borsh.Data;

function skipNBytes(Borsh.Data memory data, uint skipBytesCount) internal pure {
data.requireSpace(skipBytesCount);
unchecked {
data.ptr += skipBytesCount;
}
}

function skipArray(Borsh.Data memory data, uint32 itemSizeInBytes) internal pure {
uint32 itemsCount = data.decodeU32();
uint32 skipBytesCount = itemsCount * itemSizeInBytes;

data.skipNBytes(skipBytesCount);
}

function skipBytesArray(Borsh.Data memory data) internal pure {
uint32 itemCount = data.decodeU32();
for (uint32 i = 0; i < itemCount; i++) {
data.skipBytes();
}
}

function skipMerklePath(Borsh.Data memory data) internal pure {
uint32 MerklePathItemSize = 32 + 1;
data.skipArray(MerklePathItemSize);
}

function skipExecutionStatus(Borsh.Data memory data) internal pure {
uint8 enumIndex = data.decodeU8();
if (enumIndex == 2) {
data.skipBytes();
} else if (enumIndex == 3) {
data.skipNBytes(32);
}
}

function skipExecutionOutcomeWithIdAndProof(Borsh.Data memory data) internal pure {
//proof
data.skipMerklePath();

//block_hash(bytes32) + outcome.id(bytes32)
data.skipNBytes(32 + 32);

//logs
data.skipBytesArray();

//receipt_ids(bytes32[])
data.skipArray(32);

//gas_burnt(uint64) + tokens_burnt(uint128)
data.skipNBytes(8 + 16);

//executor_id
data.skipBytes();

data.skipExecutionStatus();
}

function getBlockHeightFromProof(bytes calldata proofData) internal pure returns(uint64) {
Borsh.Data memory data = Borsh.from(proofData);

//outcome_proof
data.skipExecutionOutcomeWithIdAndProof();

//outcome_root_proof
data.skipMerklePath();

// prev_block_hash(bytes32) + inner_rest_hash(bytes32)
data.skipNBytes(32 + 32);

return data.decodeU64();
}
}
32 changes: 27 additions & 5 deletions eth-custodian/contracts/EthCustodianProxy.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import '@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol'
import '@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol';
import '@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol';
import '@openzeppelin/contracts-upgradeable/utils/PausableUpgradeable.sol';
import './BlockHeightFromProofExtractor.sol';
import {EthCustodian} from './EthCustodian.sol';
import {ProofKeeperGap} from './ProofKeeperGap.sol';
import {SelectivePausableUpgradable} from './SelectivePausableUpgradable.sol';
Expand Down Expand Up @@ -93,19 +94,30 @@ contract EthCustodianProxy is
);
}

/// Withdraws the appropriate amount of ETH which is encoded in `proofData`
/// * `proofData` -- this is the proof that a tokens were burned on Near.
/// * `proofBlockHeight` -- this is the block height relative to which the proof is constructed.
/// Note that the height of this block can be significantly different
/// from the block number in which the tokens were burned on Near.
/// * `receiptBlockHeight` -- the block height at which the tokens were burned on Near.
/// Should be equal to the block height in proofData.
/// Checked only for `receiptBlockHeight < <= migrationBlockHeight` for gas optimization.
/// If the tokens were burned before migration
/// the proofProducer will be updated accordingly.
function withdraw(
bytes calldata proofData,
uint64 proofBlockHeight
uint64 proofBlockHeight,
uint64 receiptBlockHeight
) external {
if (proofBlockHeight > migrationBlockHeight) {
_requireNotPaused(PAUSED_WITHDRAW_POST_MIGRATION);
ethCustodianImpl.withdraw(proofData, proofBlockHeight);
} else {
if (isPreMigration(proofData, receiptBlockHeight)) {
_requireNotPaused(PAUSED_WITHDRAW_PRE_MIGRATION);
bytes memory postMigrationProducer = ethCustodianImpl.nearProofProducerAccount_();
_writeProofProducerSlot(preMigrationProducerAccount);
ethCustodianImpl.withdraw(proofData, proofBlockHeight);
_writeProofProducerSlot(postMigrationProducer);
} else {
_requireNotPaused(PAUSED_WITHDRAW_POST_MIGRATION);
ethCustodianImpl.withdraw(proofData, proofBlockHeight);
}
}

Expand Down Expand Up @@ -149,6 +161,16 @@ contract EthCustodianProxy is
_pause(flags);
}

function isPreMigration(bytes calldata proofData, uint64 receiptBlockHeight) internal view returns(bool) {
if (receiptBlockHeight <= migrationBlockHeight) {
require(BlockHeightFromProofExtractor.getBlockHeightFromProof(proofData) == receiptBlockHeight,
'Incorrect receiptBlockHeight');
return true;
}

return false;
}

/**
* @dev Internal function called by the proxy contract to authorize an upgrade to a new implementation address
* using the UUPS proxy upgrade pattern. Overrides the default `_authorizeUpgrade` function from the `UUPSUpgradeable` contract.
Expand Down
93 changes: 72 additions & 21 deletions eth-custodian/test/EthCustodianProxy.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ const { ethers, upgrades } = require("hardhat");
const { expect } = require('chai');
const { serialize } = require('rainbow-bridge-lib/rainbow/borsh.js');
const { borshifyOutcomeProof } = require('rainbow-bridge-lib/rainbow/borshify-proof.js');
const proof = require("./proof_template_from_testnet.json");

const UNPAUSED_ALL = 0;
const PAUSED_DEPOSIT_TO_EVM = 1 << 0;
Expand Down Expand Up @@ -32,7 +33,7 @@ describe('EthCustodianProxy contract', () => {

const nearEvmAccount = Buffer.from('v1.eth-connector.testnet');
const newProofProducerData = Buffer.from('new-producer.testnet');
const migrationBlock = 19672697;
const blockHeightFromProof = 39884271;

beforeEach(async () => {
[adminAccount, ethRecipientOnNear, user1, user2] = await ethers.getSigners();
Expand Down Expand Up @@ -139,25 +140,25 @@ describe('EthCustodianProxy contract', () => {
});

it('Should pause withdraw', async () => {
await ethCustodianProxy.migrateToNewProofProducer(newProofProducerData, migrationBlock);
await ethCustodianProxy.migrateToNewProofProducer(newProofProducerData, blockHeightFromProof - 1);
await ethCustodianProxy.pauseProxy(PAUSED_WITHDRAW_POST_MIGRATION);
const proof = require('./proof_template_from_testnet.json');

await expect(ethCustodianProxy.withdraw(borshifyOutcomeProof(proof), migrationBlock + 1))
await expect(ethCustodianProxy.withdraw(borshifyOutcomeProof(proof), blockHeightFromProof + 1, blockHeightFromProof))
.to.be.revertedWith('Pausable: paused');
});

it('Should pause withdraw pre-migration', async () => {
await ethCustodianProxy.migrateToNewProofProducer(newProofProducerData, migrationBlock);
await ethCustodianProxy.migrateToNewProofProducer(newProofProducerData, blockHeightFromProof + 1);
await ethCustodianProxy.pauseProxy(PAUSED_WITHDRAW_PRE_MIGRATION);
const proof = require('./proof_template_from_testnet.json');

await expect(ethCustodianProxy.withdraw(borshifyOutcomeProof(proof), 1099))
await expect(ethCustodianProxy.withdraw(borshifyOutcomeProof(proof), blockHeightFromProof, blockHeightFromProof))
.to.be.revertedWith('Pausable: paused');
});

it('Should pause all', async () => {
await ethCustodianProxy.migrateToNewProofProducer(newProofProducerData, migrationBlock);
await ethCustodianProxy.migrateToNewProofProducer(newProofProducerData, blockHeightFromProof - 1);
await ethCustodianProxy.pauseAll();

await expect(ethCustodianProxy.depositToNear('recipient.near', 0, { value: 50000 }))
Expand All @@ -168,10 +169,10 @@ describe('EthCustodianProxy contract', () => {

const proof = require('./proof_template_from_testnet.json');

await expect(ethCustodianProxy.withdraw(borshifyOutcomeProof(proof), 1099))
await expect(ethCustodianProxy.withdraw(borshifyOutcomeProof(proof), 1099, blockHeightFromProof))
.to.be.revertedWith('Pausable: paused');

await expect(ethCustodianProxy.withdraw(borshifyOutcomeProof(proof), migrationBlock + 1))
await expect(ethCustodianProxy.withdraw(borshifyOutcomeProof(proof), blockHeightFromProof + 1, blockHeightFromProof))
.to.be.revertedWith('Pausable: paused');
});
});
Expand All @@ -180,10 +181,10 @@ describe('EthCustodianProxy contract', () => {
it('Should change proof producer for EthCustodian', async () => {
const oldProofProducer = await ethCustodian.nearProofProducerAccount_();

await ethCustodianProxy.migrateToNewProofProducer(newProofProducerData, migrationBlock);
await ethCustodianProxy.migrateToNewProofProducer(newProofProducerData, blockHeightFromProof);

expect(await ethCustodianProxy.migrationBlockHeight())
.to.equal(migrationBlock);
.to.equal(blockHeightFromProof);

expect(await ethCustodianProxy.preMigrationProducerAccount())
.to.equal(oldProofProducer);
Expand All @@ -193,17 +194,16 @@ describe('EthCustodianProxy contract', () => {
});

it('Should fail when invoked for the second time', async () => {
await ethCustodianProxy.migrateToNewProofProducer(newProofProducerData, migrationBlock);
await ethCustodianProxy.migrateToNewProofProducer(newProofProducerData, blockHeightFromProof);

await expect(ethCustodianProxy.migrateToNewProofProducer(newProofProducerData, migrationBlock))
await expect(ethCustodianProxy.migrateToNewProofProducer(newProofProducerData, blockHeightFromProof))
.to.be.revertedWithCustomError(ethCustodianProxy, 'AlreadyMigrated');
});

it('Should fail when block producer id is too long', async () => {
await expect(
ethCustodianProxy.migrateToNewProofProducer(Buffer.from('new-loooooooong-producer.testnet'), migrationBlock)
)
.to.be.revertedWithCustomError(ethCustodianProxy, 'ProducerAccountIdTooLong');
await expect(
ethCustodianProxy.migrateToNewProofProducer(Buffer.from('new-loooooooong-producer.testnet'), blockHeightFromProof)
).to.be.revertedWithCustomError(ethCustodianProxy, 'ProducerAccountIdTooLong');
});
});

Expand All @@ -216,8 +216,6 @@ describe('EthCustodianProxy contract', () => {
.connect(user1)
.depositToEVM(ethRecipientOnNear.address, 0, { value: 200000 });

await ethCustodianProxy.migrateToNewProofProducer(newProofProducerData, migrationBlock);

proof.outcome_proof.outcome.status.SuccessValue = serialize(SCHEMA, 'WithdrawResult', {
amount: amount,
recipient: ethers.getBytes(user2.address),
Expand All @@ -226,6 +224,8 @@ describe('EthCustodianProxy contract', () => {
});

it('Should successfully withdraw and emit the event post-migration', async () => {
await ethCustodianProxy.migrateToNewProofProducer(newProofProducerData, blockHeightFromProof - 1);

const postMigrationProof = structuredClone(proof);
postMigrationProof.outcome_proof.outcome.executor_id = 'new-producer.testnet';

Expand All @@ -234,7 +234,7 @@ describe('EthCustodianProxy contract', () => {
await ethers.provider.getBalance(user2.address));

await expect(
ethCustodianProxy.withdraw(borshifyOutcomeProof(postMigrationProof), migrationBlock + 1)
ethCustodianProxy.withdraw(borshifyOutcomeProof(postMigrationProof), blockHeightFromProof + 1, blockHeightFromProof)
)
.to.emit(ethCustodian, 'Withdrawn')
.withArgs(user2.address, amount);
Expand All @@ -247,11 +247,61 @@ describe('EthCustodianProxy contract', () => {
expect(balanceDiff).to.equal(amount)
});

it('Should successfully withdraw and emit the event: checkPreMigration switched off', async () => {
await ethCustodianProxy.migrateToNewProofProducer(newProofProducerData, blockHeightFromProof + 1);

const postMigrationProof = structuredClone(proof);
postMigrationProof.outcome_proof.outcome.executor_id = 'new-producer.testnet';

const proofProducerBefore = await ethCustodian.nearProofProducerAccount_();
const balanceBefore = BigInt(
await ethers.provider.getBalance(user2.address));

await expect(
ethCustodianProxy.withdraw(borshifyOutcomeProof(postMigrationProof), blockHeightFromProof, blockHeightFromProof + 2)
)
.to.emit(ethCustodian, 'Withdrawn')
.withArgs(user2.address, amount);

const proofProducerAfter = await ethCustodian.nearProofProducerAccount_();
const balanceAfter = await ethers.provider.getBalance(user2.address);
const balanceDiff = balanceAfter - balanceBefore;

expect(proofProducerBefore).to.equal(proofProducerAfter);
expect(balanceDiff).to.equal(amount)
});

it('Should revert on withdraw: incorrect receiptBlockHeight', async () => {
await ethCustodianProxy.migrateToNewProofProducer(newProofProducerData, blockHeightFromProof + 1);
const balanceBefore = BigInt(await ethers.provider.getBalance(user2.address));

await expect(
ethCustodianProxy.withdraw(borshifyOutcomeProof(proof), blockHeightFromProof, blockHeightFromProof - 1)
).to.be.revertedWith('Incorrect receiptBlockHeight');
});

it('Should successfully withdraw and emit the event pre-migration with post-migration block height', async () => {
await ethCustodianProxy.migrateToNewProofProducer(newProofProducerData, blockHeightFromProof + 1);
const balanceBefore = BigInt(await ethers.provider.getBalance(user2.address));

await expect(
ethCustodianProxy.withdraw(borshifyOutcomeProof(proof), blockHeightFromProof + 2, blockHeightFromProof)
)
.to.emit(ethCustodian, 'Withdrawn')
.withArgs(user2.address, amount);

const balanceAfter = BigInt(await ethers.provider.getBalance(user2.address));
const balanceDiff = balanceAfter - balanceBefore;

expect(balanceDiff).to.equal(amount)
});

it('Should successfully withdraw and emit the event pre-migration', async () => {
await ethCustodianProxy.migrateToNewProofProducer(newProofProducerData, blockHeightFromProof + 1);
const balanceBefore = BigInt(await ethers.provider.getBalance(user2.address));

await expect(
ethCustodianProxy.withdraw(borshifyOutcomeProof(proof), 1099)
ethCustodianProxy.withdraw(borshifyOutcomeProof(proof), blockHeightFromProof, blockHeightFromProof)
)
.to.emit(ethCustodian, 'Withdrawn')
.withArgs(user2.address, amount);
Expand All @@ -264,11 +314,12 @@ describe('EthCustodianProxy contract', () => {


it('Should successfully withdraw and emit the event at migration block', async () => {
await ethCustodianProxy.migrateToNewProofProducer(newProofProducerData, blockHeightFromProof);
const balanceBefore = BigInt(await ethers.provider.getBalance(user2.address));
const migrationBlock = await ethCustodianProxy.migrationBlockHeight();

await expect(
ethCustodianProxy.withdraw(borshifyOutcomeProof(proof), parseInt(migrationBlock))
ethCustodianProxy.withdraw(borshifyOutcomeProof(proof), parseInt(migrationBlock), blockHeightFromProof)
)
.to.emit(ethCustodian, 'Withdrawn')
.withArgs(user2.address, amount);
Expand Down

0 comments on commit 02d3f22

Please sign in to comment.