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

Differentiate corresponding addresses of validators and trusted orgs #101

Merged
14 changes: 14 additions & 0 deletions contracts/libraries/Math.sol
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,18 @@ library Math {
function subNonNegative(uint256 a, uint256 b) internal pure returns (uint256) {
return a - min(a, b);
}

/**
* @dev Returns whether all pairs of value in an array of addresses is distinct.
*/
function containsNoDuplicated(address[] memory values) internal pure returns (bool) {
for (uint _i = 0; _i < values.length - 1; _i++) {
for (uint _j = _i + 1; _j < values.length; _j++) {
if (values[_i] == values[_j]) {
return false;
}
}
}
return true;
}
}
7 changes: 7 additions & 0 deletions contracts/multi-chains/RoninTrustedOrganization.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pragma solidity ^0.8.9;

import "@openzeppelin/contracts/utils/Strings.sol";
import "@openzeppelin/contracts/proxy/utils/Initializable.sol";
import "../libraries/Math.sol";
import "../interfaces/IRoninTrustedOrganization.sol";
import "../extensions/collections/HasProxyAdmin.sol";

Expand Down Expand Up @@ -264,6 +265,12 @@ contract RoninTrustedOrganization is IRoninTrustedOrganization, HasProxyAdmin, I
require(_v.addedBlock == 0, "RoninTrustedOrganization: invalid request");
require(_v.weight > 0, "RoninTrustedOrganization: invalid weight");

address[] memory _addresses = new address[](3);
_addresses[0] = _v.consensusAddr;
_addresses[1] = _v.governor;
_addresses[2] = _v.bridgeVoter;
require(Math.containsNoDuplicated(_addresses), "RoninTrustedOrganization: three addresses must be distinct");

if (_consensusWeight[_v.consensusAddr] > 0) {
revert(
string(
Expand Down
8 changes: 8 additions & 0 deletions contracts/ronin/staking/CandidateStaking.sol
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,14 @@ abstract contract CandidateStaking is BaseStaking, ICandidateStaking {
require(_sendRON(_treasuryAddr, 0), "CandidateStaking: treasury cannot receive RON");
require(_amount >= _minValidatorStakingAmount, "CandidateStaking: insufficient amount");

address[] memory _addresses = new address[](5);
_addresses[0] = _poolAdmin;
_addresses[1] = _candidateAdmin;
_addresses[2] = _consensusAddr;
_addresses[3] = _treasuryAddr;
_addresses[4] = _bridgeOperatorAddr;
require(Math.containsNoDuplicated(_addresses), "CandidateStaking: five addresses must be distinct");

_validatorContract.grantValidatorCandidate(
_candidateAdmin,
_consensusAddr,
Expand Down
2 changes: 1 addition & 1 deletion hardhat.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ const config: HardhatUserConfig = {
hardfork: 'istanbul',
accounts: {
mnemonic: DEFAULT_MNEMONIC,
count: 100,
count: 150,
accountsBalance: '1000000000000000000000000000', // 1B RON
},
allowUnlimitedContractSize: true,
Expand Down
141 changes: 92 additions & 49 deletions test/bridge/BridgeTracking.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,20 @@ import {
} from '../../src/types';
import { ERC20PresetMinterPauser } from '../../src/types/ERC20PresetMinterPauser';
import { ReceiptStruct } from '../../src/types/IRoninGatewayV2';
import {
createManyTrustedOrganizationAddressSets,
createManyValidatorCandidateAddressSets,
TrustedOrganizationAddressSet,
ValidatorCandidateAddressSet,
} from '../helpers/address-set-types';
import { initTest } from '../helpers/fixture';
import { mineBatchTxs } from '../helpers/utils';

let deployer: SignerWithAddress;
let coinbase: SignerWithAddress;
let governors: SignerWithAddress[];
let candidates: SignerWithAddress[];
let trustedOrgs: TrustedOrganizationAddressSet[];
let candidates: ValidatorCandidateAddressSet[];
let signers: SignerWithAddress[];

let bridgeContract: RoninGatewayV2;
let bridgeTracking: BridgeTracking;
Expand All @@ -51,10 +58,14 @@ const mainchainId = 1;

describe('Bridge Tracking test', () => {
before(async () => {
[deployer, coinbase, ...candidates] = await ethers.getSigners();
candidates = candidates.slice(0, maxValidatorNumber);
candidates = candidates.sort((v1, v2) => v1.address.toLowerCase().localeCompare(v2.address.toLowerCase()));
governors = candidates.slice(0, maxPrioritizedValidatorNumber);
[deployer, coinbase, ...signers] = await ethers.getSigners();
candidates = createManyValidatorCandidateAddressSets(signers.slice(0, maxValidatorNumber * 5));

trustedOrgs = createManyTrustedOrganizationAddressSets([
...signers.slice(0, maxPrioritizedValidatorNumber),
...signers.splice(maxValidatorNumber * 5, maxPrioritizedValidatorNumber),
...signers.splice(maxValidatorNumber * 5, maxPrioritizedValidatorNumber),
]);

// Deploys bridge contracts
token = await new ERC20PresetMinterPauser__factory(deployer).deploy('ERC20', 'ERC20');
Expand All @@ -80,10 +91,10 @@ describe('Bridge Tracking test', () => {
await initTest('BridgeTracking')({
bridgeContract: bridgeContract.address,
roninTrustedOrganizationArguments: {
trustedOrganizations: governors.map((v) => ({
consensusAddr: v.address,
governor: v.address,
bridgeVoter: v.address,
trustedOrganizations: trustedOrgs.map((v) => ({
consensusAddr: v.consensusAddr.address,
governor: v.governor.address,
bridgeVoter: v.bridgeVoter.address,
weight: 100,
addedBlock: 0,
})),
Expand All @@ -103,7 +114,7 @@ describe('Bridge Tracking test', () => {
governanceAdmin = RoninGovernanceAdmin__factory.connect(roninGovernanceAdminAddress, deployer);
roninValidatorSet = MockRoninValidatorSetExtended__factory.connect(validatorContractAddress, deployer);
bridgeTracking = BridgeTracking__factory.connect(bridgeTrackingAddress, deployer);
governanceAdminInterface = new GovernanceAdminInterface(governanceAdmin, ...governors);
governanceAdminInterface = new GovernanceAdminInterface(governanceAdmin, ...trustedOrgs.map((_) => _.governor));

const mockValidatorLogic = await new MockRoninValidatorSetExtended__factory(deployer).deploy();
await mockValidatorLogic.deployed();
Expand All @@ -121,12 +132,12 @@ describe('Bridge Tracking test', () => {
// Applies candidates and double check the bridge operators
for (let i = 0; i < candidates.length; i++) {
await stakingContract
.connect(candidates[i])
.connect(candidates[i].poolAdmin)
.applyValidatorCandidate(
candidates[i].address,
candidates[i].address,
candidates[i].address,
candidates[i].address,
candidates[i].candidateAdmin.address,
candidates[i].consensusAddr.address,
candidates[i].treasuryAddr.address,
candidates[i].bridgeOperator.address,
1,
{ value: minValidatorStakingAmount + candidates.length - i }
);
Expand All @@ -138,7 +149,7 @@ describe('Bridge Tracking test', () => {
await roninValidatorSet.connect(coinbase).wrapUpEpoch();
});
period = await roninValidatorSet.currentPeriod();
expect(await roninValidatorSet.getBridgeOperators()).eql(candidates.map((v) => v.address));
expect(await roninValidatorSet.getBridgeOperators()).eql(candidates.map((v) => v.bridgeOperator.address));
});

it('Should be able to get contract configs correctly', async () => {
Expand Down Expand Up @@ -169,48 +180,64 @@ describe('Bridge Tracking test', () => {
submitWithdrawalSignatures = [0, 1, 2, 3, 4, 5];
mainchainWithdrewIds = [6, 7, 8, 9, 10];

await bridgeContract.connect(candidates[0]).depositFor(receipt);
await bridgeContract.connect(candidates[0]).tryBulkAcknowledgeMainchainWithdrew(mainchainWithdrewIds);
await bridgeContract.connect(candidates[0]).bulkSubmitWithdrawalSignatures(
await bridgeContract.connect(candidates[0].bridgeOperator).depositFor(receipt);
await bridgeContract
.connect(candidates[0].bridgeOperator)
.tryBulkAcknowledgeMainchainWithdrew(mainchainWithdrewIds);
await bridgeContract.connect(candidates[0].bridgeOperator).bulkSubmitWithdrawalSignatures(
submitWithdrawalSignatures,
submitWithdrawalSignatures.map(() => [])
);

expect(await bridgeTracking.totalVotes(period)).eq(0);
expect(await bridgeTracking.totalBallots(period)).eq(0);
expect(await bridgeTracking.totalBallotsOf(period, candidates[0].address)).eq(0);
expect(await bridgeTracking.totalBallotsOf(period, candidates[0].bridgeOperator.address)).eq(0);
});

it('Should be able to record the votes/ballots when the receipts are already approved', async () => {
{
const tx = await bridgeContract.connect(candidates[1]).depositFor(receipt);
const tx = await bridgeContract.connect(candidates[1].bridgeOperator).depositFor(receipt);
await expect(tx).emit(bridgeContract, 'Deposited').withArgs(anyValue, anyValue);
}
{
const tx = await bridgeContract.connect(candidates[1]).tryBulkAcknowledgeMainchainWithdrew(mainchainWithdrewIds);
const tx = await bridgeContract
.connect(candidates[1].bridgeOperator)
.tryBulkAcknowledgeMainchainWithdrew(mainchainWithdrewIds);
await expect(tx).emit(bridgeContract, 'MainchainWithdrew').withArgs(anyValue, anyValue);
}
await bridgeContract.connect(candidates[1]).bulkSubmitWithdrawalSignatures(
await bridgeContract.connect(candidates[1].bridgeOperator).bulkSubmitWithdrawalSignatures(
submitWithdrawalSignatures,
submitWithdrawalSignatures.map(() => [])
);

// Should skips for the method `bulkSubmitWithdrawalSignatures` because no one requests these withdrawals
expect(await bridgeTracking.totalVotes(period)).eq(1 + mainchainWithdrewIds.length);
expect(await bridgeTracking.totalBallots(period)).eq((1 + mainchainWithdrewIds.length) * 2);
expect(await bridgeTracking.totalBallotsOf(period, candidates[0].address)).eq(1 + mainchainWithdrewIds.length);
expect(await bridgeTracking.totalBallotsOf(period, candidates[1].address)).eq(1 + mainchainWithdrewIds.length);
expect(await bridgeTracking.totalBallotsOf(period, candidates[0].bridgeOperator.address)).eq(
1 + mainchainWithdrewIds.length
);
expect(await bridgeTracking.totalBallotsOf(period, candidates[1].bridgeOperator.address)).eq(
1 + mainchainWithdrewIds.length
);
});

it('Should still be able to record for those who vote once the request is approved', async () => {
await bridgeContract.connect(candidates[2]).tryBulkDepositFor([receipt]);
await bridgeContract.connect(candidates[2]).tryBulkAcknowledgeMainchainWithdrew(mainchainWithdrewIds);
await bridgeContract.connect(candidates[2].bridgeOperator).tryBulkDepositFor([receipt]);
await bridgeContract
.connect(candidates[2].bridgeOperator)
.tryBulkAcknowledgeMainchainWithdrew(mainchainWithdrewIds);

expect(await bridgeTracking.totalVotes(period)).eq(1 + mainchainWithdrewIds.length);
expect(await bridgeTracking.totalBallots(period)).eq((1 + mainchainWithdrewIds.length) * 3);
expect(await bridgeTracking.totalBallotsOf(period, candidates[0].address)).eq(1 + mainchainWithdrewIds.length);
expect(await bridgeTracking.totalBallotsOf(period, candidates[1].address)).eq(1 + mainchainWithdrewIds.length);
expect(await bridgeTracking.totalBallotsOf(period, candidates[2].address)).eq(1 + mainchainWithdrewIds.length);
expect(await bridgeTracking.totalBallotsOf(period, candidates[0].bridgeOperator.address)).eq(
1 + mainchainWithdrewIds.length
);
expect(await bridgeTracking.totalBallotsOf(period, candidates[1].bridgeOperator.address)).eq(
1 + mainchainWithdrewIds.length
);
expect(await bridgeTracking.totalBallotsOf(period, candidates[2].bridgeOperator.address)).eq(
1 + mainchainWithdrewIds.length
);
});

it('Should not record in the next period', async () => {
Expand All @@ -222,23 +249,31 @@ describe('Bridge Tracking test', () => {
const newPeriod = await roninValidatorSet.currentPeriod();
expect(newPeriod).not.eq(period);

await bridgeContract.connect(candidates[3]).depositFor(receipt);
await bridgeContract.connect(candidates[3]).tryBulkAcknowledgeMainchainWithdrew(mainchainWithdrewIds);
await bridgeContract.connect(candidates[3].bridgeOperator).depositFor(receipt);
await bridgeContract
.connect(candidates[3].bridgeOperator)
.tryBulkAcknowledgeMainchainWithdrew(mainchainWithdrewIds);

expect(await bridgeTracking.totalVotes(period)).eq(1 + mainchainWithdrewIds.length);
expect(await bridgeTracking.totalBallots(period)).eq((1 + mainchainWithdrewIds.length) * 3);
expect(await bridgeTracking.totalBallotsOf(period, candidates[0].address)).eq(1 + mainchainWithdrewIds.length);
expect(await bridgeTracking.totalBallotsOf(period, candidates[1].address)).eq(1 + mainchainWithdrewIds.length);
expect(await bridgeTracking.totalBallotsOf(period, candidates[2].address)).eq(1 + mainchainWithdrewIds.length);
expect(await bridgeTracking.totalBallotsOf(period, candidates[3].address)).eq(0);
expect(await bridgeTracking.totalBallotsOf(period, candidates[0].bridgeOperator.address)).eq(
1 + mainchainWithdrewIds.length
);
expect(await bridgeTracking.totalBallotsOf(period, candidates[1].bridgeOperator.address)).eq(
1 + mainchainWithdrewIds.length
);
expect(await bridgeTracking.totalBallotsOf(period, candidates[2].bridgeOperator.address)).eq(
1 + mainchainWithdrewIds.length
);
expect(await bridgeTracking.totalBallotsOf(period, candidates[3].bridgeOperator.address)).eq(0);

period = newPeriod;
expect(await bridgeTracking.totalVotes(newPeriod)).eq(0);
expect(await bridgeTracking.totalBallots(newPeriod)).eq(0);
expect(await bridgeTracking.totalBallotsOf(newPeriod, candidates[0].address)).eq(0);
expect(await bridgeTracking.totalBallotsOf(newPeriod, candidates[1].address)).eq(0);
expect(await bridgeTracking.totalBallotsOf(newPeriod, candidates[2].address)).eq(0);
expect(await bridgeTracking.totalBallotsOf(newPeriod, candidates[3].address)).eq(0);
expect(await bridgeTracking.totalBallotsOf(newPeriod, candidates[0].bridgeOperator.address)).eq(0);
expect(await bridgeTracking.totalBallotsOf(newPeriod, candidates[1].bridgeOperator.address)).eq(0);
expect(await bridgeTracking.totalBallotsOf(newPeriod, candidates[2].bridgeOperator.address)).eq(0);
expect(await bridgeTracking.totalBallotsOf(newPeriod, candidates[3].bridgeOperator.address)).eq(0);
});

it('Should be able to request withdrawal and record voting for submitting signatures', async () => {
Expand All @@ -254,27 +289,35 @@ describe('Bridge Tracking test', () => {
mainchainId
);

await bridgeContract.connect(candidates[0]).bulkSubmitWithdrawalSignatures(
await bridgeContract.connect(candidates[0].bridgeOperator).bulkSubmitWithdrawalSignatures(
submitWithdrawalSignatures,
submitWithdrawalSignatures.map(() => [])
);
await bridgeContract.connect(candidates[1]).bulkSubmitWithdrawalSignatures(
await bridgeContract.connect(candidates[1].bridgeOperator).bulkSubmitWithdrawalSignatures(
submitWithdrawalSignatures,
submitWithdrawalSignatures.map(() => [])
);
await bridgeContract.connect(candidates[2]).bulkSubmitWithdrawalSignatures(
await bridgeContract.connect(candidates[2].bridgeOperator).bulkSubmitWithdrawalSignatures(
submitWithdrawalSignatures,
submitWithdrawalSignatures.map(() => [])
);
await bridgeContract.connect(candidates[3]).bulkSubmitWithdrawalSignatures(
await bridgeContract.connect(candidates[3].bridgeOperator).bulkSubmitWithdrawalSignatures(
submitWithdrawalSignatures,
submitWithdrawalSignatures.map(() => [])
);
expect(await bridgeTracking.totalVotes(period)).eq(submitWithdrawalSignatures.length);
expect(await bridgeTracking.totalBallots(period)).eq(submitWithdrawalSignatures.length * 4);
expect(await bridgeTracking.totalBallotsOf(period, candidates[0].address)).eq(submitWithdrawalSignatures.length);
expect(await bridgeTracking.totalBallotsOf(period, candidates[1].address)).eq(submitWithdrawalSignatures.length);
expect(await bridgeTracking.totalBallotsOf(period, candidates[2].address)).eq(submitWithdrawalSignatures.length);
expect(await bridgeTracking.totalBallotsOf(period, candidates[3].address)).eq(submitWithdrawalSignatures.length);
expect(await bridgeTracking.totalBallotsOf(period, candidates[0].bridgeOperator.address)).eq(
submitWithdrawalSignatures.length
);
expect(await bridgeTracking.totalBallotsOf(period, candidates[1].bridgeOperator.address)).eq(
submitWithdrawalSignatures.length
);
expect(await bridgeTracking.totalBallotsOf(period, candidates[2].bridgeOperator.address)).eq(
submitWithdrawalSignatures.length
);
expect(await bridgeTracking.totalBallotsOf(period, candidates[3].bridgeOperator.address)).eq(
submitWithdrawalSignatures.length
);
});
});
Loading