Skip to content
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

fix(protocol): Remove enableDestChain functionality #12341

Merged
merged 5 commits into from
Jan 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 3 additions & 21 deletions packages/bridge-ui/src/constants/abi/Bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export default [
},
{
indexed: false,
internalType: "enum LibBridgeData.MessageStatus",
internalType: "enum LibBridgeStatus.MessageStatus",
name: "status",
type: "uint8",
},
Expand Down Expand Up @@ -217,24 +217,6 @@ export default [
stateMutability: "view",
type: "function",
},
{
inputs: [
{
internalType: "uint256",
name: "_chainId",
type: "uint256",
},
{
internalType: "bool",
name: "enabled",
type: "bool",
},
],
name: "enableDestChain",
outputs: [],
stateMutability: "nonpayable",
type: "function",
},
{
inputs: [
{
Expand All @@ -246,7 +228,7 @@ export default [
name: "getMessageStatus",
outputs: [
{
internalType: "enum LibBridgeData.MessageStatus",
internalType: "enum LibBridgeStatus.MessageStatus",
name: "",
type: "uint8",
},
Expand Down Expand Up @@ -616,7 +598,7 @@ export default [
},
{
internalType: "bool",
name: "lastAttempt",
name: "isLastAttempt",
type: "bool",
},
],
Expand Down
14 changes: 2 additions & 12 deletions packages/protocol/contracts/bridge/Bridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -99,17 +99,6 @@ contract Bridge is EssentialContract, IBridge {
});
}

function enableDestChain(
uint256 _chainId,
bool enabled
) external nonReentrant {
LibBridgeSend.enableDestChain({
state: state,
chainId: _chainId,
enabled: enabled
});
}

/*********************
* Public Functions *
*********************/
Expand Down Expand Up @@ -169,6 +158,7 @@ contract Bridge is EssentialContract, IBridge {
}

function isDestChainEnabled(uint256 _chainId) public view returns (bool) {
return state.destChains[_chainId];
return
LibBridgeSend.isDestChainEnabled(AddressResolver(this), _chainId);
}
}
4 changes: 1 addition & 3 deletions packages/protocol/contracts/bridge/libs/LibBridgeData.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,9 @@ import "../IBridge.sol";
*/
library LibBridgeData {
struct State {
// chainId => isEnabled
mapping(uint256 => bool) destChains;
uint256 nextMessageId;
IBridge.Context ctx; // 3 slots
uint256[45] __gap;
uint256[46] __gap;
}

bytes32 internal constant SIGNAL_PLACEHOLDER = bytes32(uint256(1));
Expand Down
20 changes: 7 additions & 13 deletions packages/protocol/contracts/bridge/libs/LibBridgeSend.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ library LibBridgeSend {
*
* @param message Specifies the `depositValue`, `callValue`,
* and `processingFee`. These must sum to `msg.value`. It also specifies the
* `destChainId` which must be first enabled via `enableDestChain`,
* `destChainId` which must have a `bridge` address set on the AddressResolver
* and differ from the current chain ID.
*
* @return signal The message is hashed, stored, and emitted as a signal.
Expand All @@ -42,7 +42,7 @@ library LibBridgeSend {
require(message.owner != address(0), "B:owner");
require(
message.destChainId != block.chainid &&
state.destChains[message.destChainId],
isDestChainEnabled(resolver, message.destChainId),
"B:destChainId"
);

Expand All @@ -69,16 +69,10 @@ library LibBridgeSend {
emit LibBridgeData.MessageSent(signal, message);
}

/**
* Enable a destination chain ID for bridge transactions.
*/
function enableDestChain(
LibBridgeData.State storage state,
uint256 chainId,
bool enabled
) internal {
require(chainId > 0 && chainId != block.chainid, "B:chainId");
state.destChains[chainId] = enabled;
emit LibBridgeData.DestChainEnabled(chainId, enabled);
function isDestChainEnabled(
AddressResolver resolver,
uint256 chainId
) internal view returns (bool) {
return resolver.resolve(chainId, "bridge") != address(0);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,4 @@ contract TestLibBridgeSend is EssentialContract {
) public payable returns (bytes32 signal) {
return LibBridgeSend.sendMessage(state, AddressResolver(this), message);
}

function enableDestChain(uint256 chainId, bool enabled) public {
LibBridgeSend.enableDestChain(state, chainId, enabled);
}

function getDestChainStatus(uint256 chainId) public view returns (bool) {
return state.destChains[chainId];
}
}
6 changes: 0 additions & 6 deletions packages/protocol/docs/bridge/Bridge.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,6 @@ function processMessage(struct IBridge.Message message, bytes proof) external
function retryMessage(struct IBridge.Message message, bool isLastAttempt) external
```

### enableDestChain

```solidity
function enableDestChain(uint256 _chainId, bool enabled) external
```

### isMessageSent

```solidity
Expand Down
7 changes: 5 additions & 2 deletions packages/protocol/test/bridge/Bridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,6 @@ async function deployBridge(

await bridge.connect(signer).init(addressManager.address);

await bridge.connect(signer).enableDestChain(destChain, true);

const etherVault: EtherVault = await (
await ethers.getContractFactory("EtherVault")
)
Expand Down Expand Up @@ -107,6 +105,11 @@ describe("Bridge", function () {
srcChainId
);

await addressManager.setAddress(
`${enabledDestChainId}.bridge`,
"0x0000000000000000000000000000000000000001" // dummy address so chain is "enabled"
);

// deploy protocol contract
return {
owner,
Expand Down
29 changes: 5 additions & 24 deletions packages/protocol/test/bridge/libs/LibBridgeSend.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ describe("LibBridgeSend", function () {
).deploy();
await addressManager.init();

await addressManager.setAddress(
`${enabledDestChainId}.bridge`,
"0x0000000000000000000000000000000000000001" // dummy address so chain is "enabled"
);

const etherVault: EtherVault = await (
await ethers.getContractFactory("EtherVault")
)
Expand All @@ -51,26 +56,6 @@ describe("LibBridgeSend", function () {
.authorize(libSend.address, true);
});

describe("enableDestChain()", async function () {
it("should throw when chainId <= 0", async function () {
await expect(libSend.enableDestChain(0, true)).to.be.revertedWith(
"B:chainId"
);
});

it("should throw when chainId == block.chainId", async function () {
await expect(
libSend.enableDestChain(blockChainId, true)
).to.be.revertedWith("B:chainId");
});

it("should emit DestChainEnabled() event", async function () {
expect(
await libSend.enableDestChain(enabledDestChainId, true)
).to.emit(libSend, "DestChainEnabled");
});
});

describe("sendMessage()", async function () {
it("should throw when message.owner == address(0)", async function () {
const nonEnabledDestChain = 2;
Expand Down Expand Up @@ -143,8 +128,6 @@ describe("LibBridgeSend", function () {
});

it("should throw when expectedAmount != msg.value", async function () {
await libSend.enableDestChain(enabledDestChainId, true);

const message: Message = {
id: 1,
sender: owner.address,
Expand All @@ -167,8 +150,6 @@ describe("LibBridgeSend", function () {
});

it("should emit MessageSent() event and signal should be hashed correctly", async function () {
await libSend.enableDestChain(enabledDestChainId, true);

const message: Message = {
id: 1,
sender: owner.address,
Expand Down
21 changes: 16 additions & 5 deletions packages/protocol/test/genesis/generate_genesis.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ action("Generate Genesis", function () {
const addressManager = new hre.ethers.Contract(
addressManagerAlloc.address,
require("../../artifacts/contracts/thirdparty/AddressManager.sol/AddressManager.json").abi,
provider
signer
);

const owner = await addressManager.owner();
Expand All @@ -110,11 +110,11 @@ action("Generate Genesis", function () {

expect(bridge).to.be.equal(getContractAlloc("Bridge").address);

const tokenValut = await addressManager.getAddress(
const tokenVault = await addressManager.getAddress(
`${testConfig.chainId}.token_vault`
);

expect(tokenValut).to.be.equal(
expect(tokenVault).to.be.equal(
getContractAlloc("TokenVault").address
);

Expand Down Expand Up @@ -237,8 +237,6 @@ action("Generate Genesis", function () {

expect(owner).to.be.equal(testConfig.contractOwner);

await expect(Bridge.enableDestChain(1, true)).not.to.reverted;

await expect(
Bridge.processMessage(
{
Expand Down Expand Up @@ -273,6 +271,19 @@ action("Generate Genesis", function () {

expect(owner).to.be.equal(testConfig.contractOwner);

const addressManager = new hre.ethers.Contract(
getContractAlloc("AddressManager").address,
require("../../artifacts/contracts/thirdparty/AddressManager.sol/AddressManager.json").abi,
signer
);

await expect(
addressManager.setAddress(
"1.bridge",
getContractAlloc("Bridge").address
)
).not.to.be.reverted;

await expect(
TokenVault.sendEther(
1,
Expand Down
15 changes: 10 additions & 5 deletions packages/protocol/test/libs/LibTrieProof.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,18 @@ describe("integration:LibTrieProof", function () {

const { chainId } = await ethers.provider.getNetwork();

const enabledDestChainId = chainId + 1;

await addressManager.setAddress(
`${chainId}.ether_vault`,
"0xEA3dD11036f668F08940E13e3bcB097C93b09E07"
);

await addressManager.setAddress(
`${enabledDestChainId}.bridge`,
"0x0000000000000000000000000000000000000001" // dummy address so chain is "enabled"
);

const libBridgeRetry = await (
await ethers.getContractFactory("LibBridgeRetry")
).deploy();
Expand All @@ -57,23 +64,21 @@ describe("integration:LibTrieProof", function () {

const [owner] = await ethers.getSigners();

return { owner, testLibTreProof, bridge };
return { owner, testLibTreProof, bridge, enabledDestChainId };
}
describe("verify()", function () {
it("verifies", async function () {
const { owner, testLibTreProof, bridge } =
const { owner, testLibTreProof, bridge, enabledDestChainId } =
await deployLibTrieProofFixture();

const { chainId } = await ethers.provider.getNetwork();
const srcChainId = chainId;
const destChainId = srcChainId + 1;
await (await bridge.enableDestChain(destChainId, true)).wait();

const message: Message = {
id: 1,
sender: owner.address,
srcChainId: srcChainId,
destChainId: destChainId,
destChainId: enabledDestChainId,
owner: owner.address,
to: owner.address,
refundAddress: owner.address,
Expand Down
Loading