Skip to content

Conversation

@libotony
Copy link
Member

@libotony libotony commented May 8, 2025

No description provided.

@socket-security
Copy link

socket-security bot commented May 8, 2025

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedelliptic@​6.6.110010010080100

View full report

@libotony libotony requested review from fabiorigam and paologalligit and removed request for fabiorigam May 8, 2025 13:35
@coveralls
Copy link

coveralls commented May 8, 2025

Coverage Status

coverage: 96.646% (+0.3%) from 96.395%
when pulling 82c301c on tony/dynamic-fee
into db98f9c on master.

@libotony libotony requested a review from vanja-vechain May 8, 2025 13:35
Copy link

@vanja-vechain vanja-vechain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, tested on both testnet and galactica dev net. Legacy tx works as expected on testnet and both legacy and dynamic work fine on galactica dev net

@vanja-vechain
Copy link

@paologalligit this one is to be reviewed if you have some time

@paologalligit
Copy link
Member

@paologalligit this one is to be reviewed if you have some time

oh, I reviewed the pr on tony's repo 😅

@darrenvechain
Copy link

darrenvechain commented May 8, 2025

Seems the types are a bit messed up (first screenshot), missing the coef or the fees data. Wonder is there any need for generics or something (see code snippet + last screenshot)

image
export class Transaction <T extends Transaction.Body = Transaction.Body> {

    public readonly body: T
    
    constructor(body: T) {
        this.body = body
        if (body.type) {
            this.type = body.type
        } else {
            this.type = Transaction.Type.Legacy
        }
    }
image

@libotony
Copy link
Member Author

libotony commented May 9, 2025

Seems the types are a bit messed up (first screenshot), missing the coef or the fees data. Wonder is there any need for generics or something (see code snippet + last screenshot)

image ```ts export class Transaction {
public readonly body: T

constructor(body: T) {
    this.body = body
    if (body.type) {
        this.type = body.type
    } else {
        this.type = Transaction.Type.Legacy
    }
}

<img alt="image" width="885" src="https://private-user-images.githubusercontent.com/107671032/441777042-4b3e76a6-e81f-45c6-ba3b-9675f8c9bbb3.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NDY3NjU5MzcsIm5iZiI6MTc0Njc2NTYzNywicGF0aCI6Ii8xMDc2NzEwMzIvNDQxNzc3MDQyLTRiM2U3NmE2LWU4MWYtNDVjNi1iYTNiLTk2NzVmOGM5YmJiMy5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUwNTA5JTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MDUwOVQwNDQwMzdaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT1kODdiMTE5YWFkOTVjOWRhNjEyZWQ4MDg5ZmM4MDk2Y2YxODYxNDZjZjNhNTg3YTM3MWRlODBmNWQ5NGQ0ZWNlJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.07T-AyA_F87kpAZBeplcxgzsPSY5KpZztpzTksK8Kx0">

Nice catch! Appended in the new commit, please take a look.

@sonarqubecloud
Copy link

sonarqubecloud bot commented May 9, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@libotony libotony merged commit fe886f3 into master May 9, 2025
11 of 12 checks passed
@libotony libotony deleted the tony/dynamic-fee branch May 9, 2025 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants