iotaledger / entangled

enTangle'd is an amalgamation of all things Tangle
Apache License 2.0
113 stars 66 forks source link

fix: Fix transaction offset #1472

Open howjmay opened 4 years ago

howjmay commented 4 years ago

<> In transaction_serialize_to_flex_trits() the value of macro NUM_TRITS_* and NUM_FLEX_TRITS_* may not always be the same values, so we need to unify them.

Test Plan:

<<Outline how the reviewer can verify & test your changes here>>

thibault-martinez commented 4 years ago

Hi @HowJMay, the documentation of this function says that the size should be in trits, not flex_trits.

/// Inserts the contents of an array into another array starting at a given
/// index.
/// @param[in] to_flex_trits - the array to insert into
/// @param[in] to_len - the number of trits encoded in the to_flex_trits array
/// @param[in] flex_trits - the array containing the trits to copy over
/// @param[in] len - the number of trits the flex_trits array stores
/// @param[in] start - the start index in the destination array
/// @param[in] num_trits - the number of trits to copy over
/// @return size_t - the number of trits copied over
size_t flex_trits_insert(flex_trit_t *const to_flex_trits, size_t const to_len, flex_trit_t const *const flex_trits,
                         size_t const len, size_t const start, size_t const num_trits);
howjmay commented 4 years ago

However I wonder that if the sizes used here are in trits, then how could the size used in line 133 is NUM_FLEX_TRITS_SERIALIZED_TRANSACTION. Doesn't it cause bufferover flow?

thibault-martinez commented 4 years ago

This line 133 https://github.com/iotaledger/entangled/pull/1472/files#diff-fcfdd8b4cc72f7c24ae51e4061656b3aR133 ?

It looks good to me, it's only flex_trits_insert that needs size in trits.