Closed thobson88 closed 1 year ago
For this:
TimestampCommitment
into a trait in trustchain-core, extending the Commitment
trait
timestamp()
methodnew()
method signature in the trait (with expected_data
argument of type Timestamp
)~ can't be done in a trait (as methods must have &self argument).BlockTimestampCommitment
in trustchain-ion, commitment.rs)filter()
method that filters out everything from the commitment content (a Bitcoin block header) except the timestamp field.filter()
in the BlockHashCommitment
.Ok, this seems to work but the default implementation of the timestamp()
method inside the TimestampCommitment
trait is brittle:
/// A Commitment whose expected data is a Unix time.
pub trait TimestampCommitment : Commitment {
/// Gets the timestamp as a Unix time.
fn timestamp(&self) -> Timestamp {
self.expected_data()
.as_u64()
.unwrap()
.try_into()
.expect("Construction guarantees u32.")
}
}
It would be ok if we could enforce the condition that the expected_data must have type Timestamp
, but I can't see a way to do that.
Also, I haven't yet run the implementation tests.
On the plus side, the implementation of verifiable_timestamp
in IONVerifier
is now a bit simpler, and the filter()
is implemented.
Nice one, this is much better and fixes the issue!
I've made a couple of modifications:
test_block_hash_commitment()
given revision and added failing case to test_block_timestamp_commitment()
(2f7902ce02edf6858f18db243a8b9d9045784a83)TrivialCommitment
and Commitment
traits to take a generic (that defaults to Value
) for the expected_data
(556f122f1890de8ce39274001df42687d6188d3e). We can then implement these traits with the generic as a Timestamp
for the BlockTimestampCommitment
(e.g. here). The verify_content()
method has an additional json!()
on the expected data so it is always a Value
.I think this is good to go in #106 but some potential options for future:
BlockHashCommitment
to handle committing to both the timestamp and PoW hash as we can implement both Commitment<Value>
and Commitment<Timestamp>
(distinct generics)Value
e.g.:
trait Commitment {
type Data;
// ...
fn decode_candidate_data(&self) -> fn(&[u8]) -> CommitmentResult<Data>;
// ...
}
A downside of this though would be I don't think default implementations would be possible.
Done in #108
The
TrivialCommitment
trait includes (optional) support for filtering of the candidate data.In the case of a
TimestampCommitment
, the candidate data is a Bitcoin block header. Currently the expected data (a Unix timestamp) is sought inside the whole header, not just the timestamp field. This introduces a possible attack vector. An attacker could produce a header with a different timestamp but which contains, in a different field, the 32 bits matching a different timestamp. Without filtering of the candidate data, this bogus header would be accepted as valid (in the sense that the commitment would verify successfully).The same applies to the
BlockHashCommitment
(where we should filter on the Merkle root field).