Skip to content

Fix off-by-one error in attributing signatures to blocks in CLI #2559

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.

Sign up for GitHub

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

Merged
celo-ci-bot-user merged 5 commits into from
Jan 28, 2020
Merged

Fix off-by-one error in attributing signatures to blocks in CLI #2559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f14c7bc
fix off-by-one error in attributing signatures to blocks
Jan 28, 2020
805e0ab
allow --component flag to be used to select a VM
Jan 28, 2020
0ac9060
Merge refs/heads/master into victor/off-by-one
Jan 28, 2020
98407a3
Merge refs/heads/master into victor/off-by-one
Jan 28, 2020
39a028a
Merge refs/heads/master into victor/off-by-one
Jan 28, 2020
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
6 changes: 3 additions & 3 deletions packages/celotool/src/lib/port_forward.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,18 @@ const DEFAULT_COMPONENT = 'validators'

async function getPortForwardCmd(celoEnv: string, component?: string, ports = defaultPortsString) {
if (isVmBased()) {
return Promise.resolve(getVmPortForwardCmd(celoEnv, ports))
return Promise.resolve(getVmPortForwardCmd(celoEnv, component, ports))
} else {
return getKubernetesPortForwardCmd(celoEnv, component, ports)
}
}

function getVmPortForwardCmd(celoEnv: string, ports = defaultPortsString) {
function getVmPortForwardCmd(celoEnv: string, machine = 'validator-0', ports = defaultPortsString) {
const zone = fetchEnv(envVar.KUBERNETES_CLUSTER_ZONE)
// this command expects port mappings to be of the form `[localPort]:localhost:[remotePort]`
const portMappings = ports.replace(/:/g, ':localhost:').split(' ')
const portsWithFlags = portMappings.map((mapping) => `-L ${mapping}`).join(' ')
return `gcloud compute ssh --zone ${zone} ${celoEnv}-validator-0 -- -N ${portsWithFlags}`
return `gcloud compute ssh --zone ${zone} ${celoEnv}-${machine} -- -N ${portsWithFlags}`
}

async function getKubernetesPortForwardCmd(
Expand Down
6 changes: 3 additions & 3 deletions packages/cli/src/commands/validator/signed-blocks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ export default class ValidatorSignedBlocks extends BaseCommand {
const printer = new MarkPrinter(res.flags.width!)
try {
for (const block of blocks) {
const elected = await electionCache.elected(res.flags.signer, block.number)
const signed = elected && (await electionCache.signed(res.flags.signer, block))
printer.addMark(block.number, elected, signed)
const elected = await electionCache.elected(res.flags.signer, block.number - 1)
const signed = elected && (await electionCache.signedParent(res.flags.signer, block))
printer.addMark(block.number - 1, elected, signed)
}

// TODO(victor) Fix the follow flag functionality.
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/commands/validator/status.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,9 @@ export default class ValidatorStatus extends BaseCommand {
let signatures = 0
let eligible = 0
for (const block of blocks) {
if (await electionCache.elected(signer, block.number)) {
if (await electionCache.elected(signer, block.number - 1)) {
eligible++
if (await electionCache.signed(signer, block)) {
if (await electionCache.signedParent(signer, block)) {
signatures++
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/utils/election.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ export class ElectionResultsCache {

/**
* Returns true if the given signer is present in the parent aggregated seal of the given block.
* @param signer Validator signer address to check if presnt in the block.
* @param signer Validator signer address to check if present in the block.
* @param block The block to check for a signature on.
*/
async signed(signer: Address, block: Block): Promise<boolean> {
async signedParent(signer: Address, block: Block): Promise<boolean> {
const electedSigners = await this.electedSigners(block.number)
const signerIndex = electedSigners.map(eqAddress.bind(null, signer)).indexOf(true)
if (signerIndex < 0) {
Expand Down