0-indexed Checkpoints (#344)

* Checkpoints are zero indexed

* Checkpoints are not one-indexed anymore

* Agent changes for 0-indexed checkpoints

* Disallow checkpoint index 0

* Can just check indicies again

* Proper isCheckpoint

* Dispatch dummy message

* fix before

* PR review

Co-authored-by: Asa Oines <asaoines@Asas-MacBook-Pro.local>
pull/360/head
Nam Chu Hoai 3 years ago committed by GitHub
parent 47ce61ab62
commit e5ea4d42a9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 5
      rust/agents/checkpointer/src/submit.rs
  2. 15
      rust/agents/relayer/src/checkpoint_relayer.rs
  3. 8
      rust/agents/relayer/src/merkle_tree_builder.rs
  4. 14
      solidity/apps/test/governance/governanceRouter.test.ts
  5. 4
      solidity/core/contracts/Common.sol
  6. 10
      solidity/core/contracts/Outbox.sol
  7. 23
      solidity/core/test/outbox.test.ts
  8. 6
      solidity/core/test/validator-manager/outboxValidatorManager.test.ts
  9. 22
      typescript/hardhat/src/TestAbacusDeploy.ts

@ -49,8 +49,9 @@ impl CheckpointSubmitter {
"Got latest checkpoint and count" "Got latest checkpoint and count"
); );
// If there are any new messages, the count will be greater than // If there are any new messages, the count will be greater than
// the latest checkpoint index. // the latest checkpoint index + 1. Don't checkpoint for count < 2
if count > latest_checkpoint_index { // since checkpoints with index 0 are disallowed
if count > latest_checkpoint_index + 1 && count > 1 {
debug!("Creating checkpoint"); debug!("Creating checkpoint");
self.outbox.create_checkpoint().await?; self.outbox.create_checkpoint().await?;
// Sleep to ensure that another checkpoint isn't made until // Sleep to ensure that another checkpoint isn't made until

@ -40,7 +40,7 @@ impl CheckpointRelayer {
} }
/// Only gets the messages desinated for the Relayers inbox /// Only gets the messages desinated for the Relayers inbox
/// Exclusive the to_checkpoint_index /// Inclusive the to_checkpoint_index
async fn get_messages_between( async fn get_messages_between(
&self, &self,
from_leaf_index: u32, from_leaf_index: u32,
@ -48,7 +48,7 @@ impl CheckpointRelayer {
) -> Result<Option<Vec<CommittedMessage>>> { ) -> Result<Option<Vec<CommittedMessage>>> {
let mut messages: Vec<CommittedMessage> = vec![]; let mut messages: Vec<CommittedMessage> = vec![];
let mut current_leaf_index = from_leaf_index; let mut current_leaf_index = from_leaf_index;
while current_leaf_index < to_checkpoint_index { while current_leaf_index <= to_checkpoint_index {
// Relies on the indexer finding this message eventually // Relies on the indexer finding this message eventually
self.db.wait_for_leaf(current_leaf_index).await?; self.db.wait_for_leaf(current_leaf_index).await?;
let maybe_message = self let maybe_message = self
@ -138,8 +138,11 @@ impl CheckpointRelayer {
let latest_inbox_checkpoint = let latest_inbox_checkpoint =
self.inbox_contracts.inbox.latest_checkpoint(None).await?; self.inbox_contracts.inbox.latest_checkpoint(None).await?;
let mut onchain_checkpoint_index = latest_inbox_checkpoint.index; let mut onchain_checkpoint_index = latest_inbox_checkpoint.index;
// Checkpoints are 1-indexed, while leaves are 0-indexed let mut next_inbox_leaf_index = if onchain_checkpoint_index == 0 {
let mut next_inbox_leaf_index = onchain_checkpoint_index; 0
} else {
onchain_checkpoint_index + 1
};
loop { loop {
sleep(Duration::from_secs(self.polling_interval)).await; sleep(Duration::from_secs(self.polling_interval)).await;
@ -161,11 +164,11 @@ impl CheckpointRelayer {
{ {
None => debug!("Couldn't fetch the relevant messages, retry this range"), None => debug!("Couldn't fetch the relevant messages, retry this range"),
Some(messages) if messages.is_empty() => { Some(messages) if messages.is_empty() => {
next_inbox_leaf_index = signed_checkpoint_index; next_inbox_leaf_index = signed_checkpoint_index + 1;
debug!("New checkpoint does not include messages for inbox") debug!("New checkpoint does not include messages for inbox")
} }
Some(messages) => { Some(messages) => {
next_inbox_leaf_index = signed_checkpoint_index; next_inbox_leaf_index = signed_checkpoint_index + 1;
debug!( debug!(
len = messages.len(), len = messages.len(),
"Signed checkpoint allows for processing of new messages" "Signed checkpoint allows for processing of new messages"

@ -144,7 +144,7 @@ impl MerkleTreeBuilder {
return Ok(()); return Ok(());
} }
let starting_index = self.prover.count() as u32; let starting_index = self.prover.count() as u32;
for i in starting_index..checkpoint.index { for i in starting_index..=checkpoint.index {
self.db.wait_for_leaf(i).await?; self.db.wait_for_leaf(i).await?;
self.ingest_leaf_index(i)?; self.ingest_leaf_index(i)?;
} }
@ -168,7 +168,7 @@ impl MerkleTreeBuilder {
&mut self, &mut self,
batch: &MessageBatch, batch: &MessageBatch,
) -> Result<(), MerkleTreeBuilderError> { ) -> Result<(), MerkleTreeBuilderError> {
if self.prover.count() as u32 > batch.current_checkpoint_index { if self.prover.count() as u32 > batch.current_checkpoint_index + 1 {
error!("Prover was already ahead of MessageBatch, something went wrong"); error!("Prover was already ahead of MessageBatch, something went wrong");
return Err(MerkleTreeBuilderError::UnexpectedProverState { return Err(MerkleTreeBuilderError::UnexpectedProverState {
prover_count: self.prover.count() as u32, prover_count: self.prover.count() as u32,
@ -185,8 +185,8 @@ impl MerkleTreeBuilder {
count = self.prover.count(), count = self.prover.count(),
"update_from_batch fast forward" "update_from_batch fast forward"
); );
// prove the until target (checkpoints are 1-indexed) // prove the until target
for i in (batch.current_checkpoint_index + 1)..batch.target_checkpoint.index { for i in (batch.current_checkpoint_index + 1)..=batch.target_checkpoint.index {
self.ingest_leaf_index(i)?; self.ingest_leaf_index(i)?;
} }

@ -21,13 +21,14 @@ describe('GovernanceRouter', async () => {
remote: GovernanceRouter, remote: GovernanceRouter,
testSet: TestSet, testSet: TestSet,
governance: GovernanceDeploy; governance: GovernanceDeploy;
let outbox: Outbox;
let interchainGasPaymaster: InterchainGasPaymaster;
before(async () => { before(async () => {
[governor, recoveryManager] = await ethers.getSigners(); [governor, recoveryManager] = await ethers.getSigners();
const testSetFactory = new TestSet__factory(governor); const testSetFactory = new TestSet__factory(governor);
testSet = await testSetFactory.deploy(); testSet = await testSetFactory.deploy();
await abacus.deploy(domains, governor);
}); });
beforeEach(async () => { beforeEach(async () => {
@ -40,10 +41,13 @@ describe('GovernanceRouter', async () => {
address: governor.address, address: governor.address,
}, },
}; };
await abacus.deploy(domains, governor);
governance = new GovernanceDeploy(config); governance = new GovernanceDeploy(config);
await governance.deploy(abacus); await governance.deploy(abacus);
router = governance.router(localDomain); router = governance.router(localDomain);
remote = governance.router(remoteDomain); remote = governance.router(remoteDomain);
outbox = abacus.outbox(localDomain);
interchainGasPaymaster = abacus.interchainGasPaymaster(localDomain);
}); });
it('Cannot be initialized twice', async () => { it('Cannot be initialized twice', async () => {
@ -425,14 +429,6 @@ describe('GovernanceRouter', async () => {
describe('interchain gas payments for dispatched messages', async () => { describe('interchain gas payments for dispatched messages', async () => {
const testInterchainGasPayment = 123456789; const testInterchainGasPayment = 123456789;
let outbox: Outbox;
let interchainGasPaymaster: InterchainGasPaymaster;
before(() => {
outbox = abacus.outbox(localDomain);
interchainGasPaymaster = abacus.interchainGasPaymaster(localDomain);
});
it('allows interchain gas payment for remote calls', async () => { it('allows interchain gas payment for remote calls', async () => {
const leafIndex = await outbox.count(); const leafIndex = await outbox.count();
const call = formatCall(testSet, 'set', [13]); const call = formatCall(testSet, 'set', [13]);

@ -21,6 +21,8 @@ abstract contract Common is ICommon, OwnableUpgradeable {
// ============ Public Variables ============ // ============ Public Variables ============
// Checkpoints of root => leaf index // Checkpoints of root => leaf index
// Checkpoints of index 0 have to be disallowed as the existence of such
// a checkpoint cannot be distinguished from their non-existence
mapping(bytes32 => uint256) public checkpoints; mapping(bytes32 => uint256) public checkpoints;
// The latest checkpointed root // The latest checkpointed root
bytes32 public checkpointedRoot; bytes32 public checkpointedRoot;
@ -121,7 +123,7 @@ abstract contract Common is ICommon, OwnableUpgradeable {
/** /**
* @notice Store the provided checkpoint. * @notice Store the provided checkpoint.
* @param _root The merkle root * @param _root The merkle root
* @param _index The next available leaf index of the merkle tree. * @param _index The leaf index of the latest message in the merkle tree.
*/ */
function _checkpoint(bytes32 _root, uint256 _index) internal { function _checkpoint(bytes32 _root, uint256 _index) internal {
checkpoints[_root] = _index; checkpoints[_root] = _index;

@ -133,14 +133,16 @@ contract Outbox is IOutbox, Version0, MerkleTreeManager, Common {
/** /**
* @notice Checkpoints the latest root and index. * @notice Checkpoints the latest root and index.
* Validators are expected to sign this checkpoint so that it can be * Validators are expected to sign this checkpoint so that it can be
* relayed to the Inbox contracts. * relayed to the Inbox contracts. Checkpoints for a single message (i.e.
* count = 1) are disallowed since they make checkpoint tracking more
* difficult.
* @dev emits Checkpoint event * @dev emits Checkpoint event
*/ */
function checkpoint() external override notFailed { function checkpoint() external override notFailed {
uint256 count = count(); uint256 count = count();
require(count > 0, "!count"); require(count > 1, "!count");
bytes32 root = root(); bytes32 root = root();
_checkpoint(root, count); _checkpoint(root, count - 1);
} }
/** /**
@ -166,7 +168,7 @@ contract Outbox is IOutbox, Version0, MerkleTreeManager, Common {
override override
returns (bool) returns (bool)
{ {
// Checkpoint indices are one-indexed. // Checkpoints are zero-indexed, but checkpoints of index 0 are disallowed
return _index > 0 && checkpoints[_root] == _index; return _index > 0 && checkpoints[_root] == _index;
} }

@ -123,16 +123,31 @@ describe('Outbox', async () => {
}); });
it('Checkpoints the latest root', async () => { it('Checkpoints the latest root', async () => {
const message = ethers.utils.formatBytes32String('message');
const count = 2;
for (let i = 0; i < count; i++) {
await outbox.dispatch(
destDomain,
utils.addressToBytes32(recipient.address),
message,
);
}
await outbox.checkpoint();
const [root, index] = await outbox.latestCheckpoint();
expect(root).to.not.equal(ethers.constants.HashZero);
expect(index).to.equal(count - 1);
expect(await outbox.isCheckpoint(root, index)).to.be.true;
});
it('does not allow a checkpoint of index 0', async () => {
const message = ethers.utils.formatBytes32String('message'); const message = ethers.utils.formatBytes32String('message');
await outbox.dispatch( await outbox.dispatch(
destDomain, destDomain,
utils.addressToBytes32(recipient.address), utils.addressToBytes32(recipient.address),
message, message,
); );
await outbox.checkpoint(); await expect(outbox.checkpoint()).to.be.revertedWith('!count');
const [root, index] = await outbox.latestCheckpoint();
expect(root).to.not.equal(ethers.constants.HashZero);
expect(index).to.equal(1);
}); });
it('Correctly calculates destinationAndNonce', async () => { it('Correctly calculates destinationAndNonce', async () => {

@ -85,6 +85,12 @@ describe('OutboxValidatorManager', () => {
it('reverts if the checkpoint is not improper', async () => { it('reverts if the checkpoint is not improper', async () => {
const message = `0x${Buffer.alloc(10).toString('hex')}`; const message = `0x${Buffer.alloc(10).toString('hex')}`;
// Send two messages to allow checkpointing
await outbox.dispatch(
INBOX_DOMAIN,
utils.addressToBytes32(signer.address),
message,
);
await outbox.dispatch( await outbox.dispatch(
INBOX_DOMAIN, INBOX_DOMAIN,
utils.addressToBytes32(signer.address), utils.addressToBytes32(signer.address),

@ -13,6 +13,7 @@ import {
AbacusConnectionManager__factory, AbacusConnectionManager__factory,
} from "@abacus-network/core"; } from "@abacus-network/core";
import { TestDeploy } from "./TestDeploy"; import { TestDeploy } from "./TestDeploy";
import { addressToBytes32 } from "@abacus-network/utils/dist/src/utils";
export type TestAbacusConfig = { export type TestAbacusConfig = {
signer: Record<types.Domain, ethers.Signer>; signer: Record<types.Domain, ethers.Signer>;
@ -98,6 +99,13 @@ export class TestAbacusDeploy extends TestDeploy<
inboxes[remote] = inbox; inboxes[remote] = inbox;
}); });
await Promise.all(deploys); await Promise.all(deploys);
// dispatch a dummy event to allow a consumer to checkpoint/process a single message
await outbox.dispatch(
remotes.find((_) => _ !== domain)!,
addressToBytes32(ethers.constants.AddressZero),
"0x"
);
return { return {
outbox, outbox,
abacusConnectionManager, abacusConnectionManager,
@ -162,8 +170,14 @@ export class TestAbacusDeploy extends TestDeploy<
new Map(); new Map();
const outbox = this.outbox(origin); const outbox = this.outbox(origin);
const [, checkpointedIndex] = await outbox.latestCheckpoint(); const [, checkpointedIndex] = await outbox.latestCheckpoint();
const latestIndex = await outbox.count(); const messageCount = await outbox.count();
if (latestIndex.eq(checkpointedIndex)) return responses; // Message count does allow for a checkpoint
if (messageCount.lte(checkpointedIndex.add(1))) return responses;
// Can't checkpoint a single message
if (messageCount.toNumber() <= 1) {
return responses;
}
await outbox.checkpoint(); await outbox.checkpoint();
const [root, index] = await outbox.latestCheckpoint(); const [root, index] = await outbox.latestCheckpoint();
@ -183,6 +197,10 @@ export class TestAbacusDeploy extends TestDeploy<
const inbox = this.inbox(origin, destination); const inbox = this.inbox(origin, destination);
const status = await inbox.messages(dispatch.args.messageHash); const status = await inbox.messages(dispatch.args.messageHash);
if (status !== types.MessageStatus.PROCESSED) { if (status !== types.MessageStatus.PROCESSED) {
if (dispatch.args.leafIndex.toNumber() == 0) {
// disregard the dummy message
continue;
}
const response = await inbox.testProcess( const response = await inbox.testProcess(
dispatch.args.message, dispatch.args.message,
dispatch.args.leafIndex.toNumber() dispatch.args.leafIndex.toNumber()

Loading…
Cancel
Save