Conversation
dmidem
left a comment
dmidem
left a comment
There was a problem hiding this comment.
I've reviewed the changes and have a couple of minor suggestions below, which don't necessarily need to be addressed in this PR but could be handled in an additional one.
| .notes | ||
| .iter() | ||
| .try_fold(ValueSum::zero(), |value_sum, ¬e| { | ||
| .try_fold(NoteValue::zero(), |value_sum, ¬e| { |
There was a problem hiding this comment.
Since the NoteValue type is now used for total supply calculations instead of ValueSum, the Error variant name ValueSumOverflow might be a bit confusing. It still refers to the sum of values, but renaming it to ValueOverflow could potentially improve clarity.
Suggestion:
In issuance.rs:640, consider renaming ValueSumOverflow to ValueOverflow:
pub enum Error {
// ...
ValueOverflow,
}Additionally, all occurrences of ValueSumOverflow in the code and comments need to be renamed to ValueOverflow.
src/issuance.rs
Outdated
| impl<T: IssueAuth> IssueBundle<T> { | ||
| /// Returns the reference notes for the `IssueBundle`. | ||
| pub fn get_reference_notes(self) -> HashMap<AssetBase, Note> { | ||
| pub fn get_reference_notes(&self) -> HashMap<AssetBase, Note> { |
There was a problem hiding this comment.
I noticed that the get_reference_notes function was introduced in PR #124 within a new impl<T: IssueAuth> IssueBundle<T> { ... } block. Since there's already the same impl block above (issuance.rs#L205), maybe we could move get_reference_notes to the existing block to consolidate related implementations.
|
Additionally, I would like to place here a remark for future consideration: to make it easier to reuse |
Co-authored-by: Dmitry Demin <dmidem@users.noreply.github.com>
…e type of the value was changed from ValueSum to NoteValue
…ntu package to be installed
…nfig Ubuntu package to be installed" This reverts commit c30e108.
…ich is required by the yeslogic-fontconfig-sys crate (a transitive dev dependency). The CI error started occurring after GitHub updated ubuntu-latest from Ubuntu 22.04 to 24.04.
…s field from SupplyInfo, add and use get_reference_note method to IssueAction
…t_reference_note, fix the tests accordingly
|
Closed in favour of #133 |
This PR is an updated copy of #128: > The amount in AssetSupply must be a NoteValue (u64) and not a ValueSum (i128). > We add reference_notes into SupplyInfo. It is a hashmap of asset bases to their respective reference note The code was updated, `get_reference_note` method of `IssueAction` was added and used, `get_reference_notes` was removed. Also, instead of a separate new `reference_notes` `HashMap` in `SupplyInfo` a new field `reference_note` was added to `AssetSupply` struct. Unit tests were fixed accordingly. --------- Co-authored-by: Constance Beguier <constance@qed-it.com> Co-authored-by: Paul <3682187+PaulLaux@users.noreply.github.com>
3 participants
The amount in
AssetSupplymust be aNoteValue(u64) and not aValueSum(i128).We add
reference_notesintoSupplyInfo. It is a hashmap of asset bases to their respective reference note.