Skip to content

Commit

Permalink
No Agora override for SCPDriver::computeHashNode
Browse files Browse the repository at this point in the history
The Agora `override` had been added to provide randomness by using the
hash of the previous block header. However, the `prev_value` param
passed to `nominate` can achieve the same result. With the advantage of
preventing a potential race condition where the ledger height is not one
less than the `slot_idx` when executing `computeHashNode`. This could
occur if a block is fetched by catchup during a nomination round.
  • Loading branch information
hewison-chris committed Jan 18, 2022
1 parent 7fdff99 commit 14459dd
Showing 1 changed file with 13 additions and 42 deletions.
55 changes: 13 additions & 42 deletions source/agora/consensus/protocol/Nominator.d
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,11 @@ public extern (C++) class Nominator : SCPDriver
/// Enrollment manager
public EnrollmentManager enroll_man;

/// hash of previous block
private Hash prev_value_hash;

/// Used as "prev_value" when nominating
private const Value empty_value;
private Value prev_value;

/// User configured nomination frequency
private const Duration nomination_interval;
Expand Down Expand Up @@ -184,7 +187,6 @@ extern(D):
this.params = params;
this.clock = clock;
this.network = network;
this.empty_value = ConsensusData.init.serializeFull().toVec();
this.kp = key_pair;
this.taskman = taskman;
this.ledger = ledger;
Expand Down Expand Up @@ -496,8 +498,6 @@ extern(D):
log.info("Nominating {} at {}", data.prettify, cur_time);
this.is_nominating = true;

// note: we are not passing the previous tx set as we don't really
// need it at this point (might later be necessary for chain upgrades)
this.nominate(slot_idx, data);
}

Expand Down Expand Up @@ -535,11 +535,19 @@ extern(D):
log.info("{}(): Proposing tx set for slot {}, ledger is at height {}",
__FUNCTION__, slot_idx, this.ledger.lastBlock().header.height);

const lastBlockHash = this.ledger.lastBlock.hashFull();
if (prev_value_hash != lastBlockHash)
{
// Use hash of previous block as the previous slot value for randomizing
prev_value = lastBlockHash.serializeFull().toVec();
prev_value_hash = lastBlockHash;
}

auto next_value = next.serializeFull().toVec();
auto next_dup = duplicate_value(&next_value);
auto nextval = this.wrapValue(next_dup);

if (this.scp.nominate(slot_idx, nextval, this.empty_value))
if (this.scp.nominate(slot_idx, nextval, prev_value))
{
log.info("{}(): Tx set triggered new nomination", __FUNCTION__);
}
Expand Down Expand Up @@ -1323,43 +1331,6 @@ extern(D):
timeout.msecs, { callCPPDelegate(callback); });
}

/***************************************************************************
Used by the nomination protocol to randomize the order of messages
between nodes.
Params:
slot_idx = the slot index we're currently reaching consensus for.
prev = the previous data set for the provided slot index.
is_priority = the flag to check that this call is for priority.
round_num = the nomination round
node_id = the id of the node for which this computation is being made
Returns:
the 8-byte hash
***************************************************************************/

public override uint64_t computeHashNode (uint64_t slot_idx,
ref const(Value) prev, bool is_priority, int32_t round_num,
ref const(NodeID) node_id) nothrow
{
const uint hash_N = 1;
const uint hash_P = 2;

const header = this.ledger.lastBlock().header;
assert(header.height + 1 == slot_idx); // Not sure if this should ever happen
const seed = header.hashFull();
const Hash hash = hashMulti(slot_idx, prev[],
is_priority ? hash_P : hash_N, round_num, node_id, seed);

uint64_t res = 0;
for (size_t i = 0; i < res.sizeof; i++)
res = (res << 8) | hash[][i];

return res;
}

// `getHashOf` computes the hash for the given vector of byte vector
override StellarHash getHashOf (ref vector!Value vals) const nothrow
{
Expand Down

0 comments on commit 14459dd

Please sign in to comment.