Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

retrying to implement exact expiry #47

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arkanoider
Copy link

Hi @zoedberg ,

i am back, will do another tentative :)

I am restarting from scratch following the old ideas you exposed me in PR, this is not compiling and still here to ask you if i am doing right.

First of all i had a compiler error that on my machine seems created by the missing of this feature:

https://github.com/arkanoider/rgb-lib/blob/06bdd89cc5c079f049a8d195a4f80ca99e5d1cda/migration/Cargo.toml#L29

when i raised this db commands ( the second one if I remember correctly in particular) :


docker run -p 127.0.0.1:5432:5432/tcp --name migration-postgres \
    -e POSTGRES_PASSWORD=mysecretpassword -d postgres

DATABASE_URL=postgres://postgres:mysecretpassword@localhost:5432 \
    sea-orm-cli migrate refresh

DATABASE_URL=postgres://postgres:mysecretpassword@localhost:5432/postgres \
    sea-orm-cli generate entity -o src/database/entities --expanded-format

docker rm -f migration-postgres

After this I started updating the test_witness_receive with the false boolean for exact expiry, but there's a point where going down in functions call seems to me that i need to update this structure:

#[derive(Clone, Eq, PartialEq, Debug)]
pub struct RgbInvoice {
    pub transports: Vec<RgbTransport>,
    pub contract: Option<ContractId>,
    pub iface: Option<TypeName>,
    pub operation: Option<TypeName>,
    pub assignment: Option<FieldName>,
    pub beneficiary: Beneficiary,
    pub owned_state: TypedState,
    pub chain: Option<Chain>,
    /// UTC unix timestamp
    pub expiry: Option<i64, bool>,
    pub unknown_query: IndexMap<String, String>,
}

called inside _receive method. This is placed in the crate rgb-wallet so maybe I should also touch that structure, but maybe I am wrong...

Can you confirm me that's the right path?

Still sorry for my noobnees, but I am trying to learn more about rgb, but my time is scarse, so I am more going around the code to learn, but I am missing a bit of the bigger scenario of rgb.

Thanks for your patience!

@zoedberg zoedberg force-pushed the master branch 12 times, most recently from eaa7fe0 to c4fdedd Compare April 10, 2024 15:49
@zoedberg
Copy link
Collaborator

Hi @arkanoider, we've just released 0.3.0-alpha.1 with support for RGB 0.11 beta 5 so I suggest rebasing this on top of that before proceeding (a lot of things changed).

First of all i had a compiler error that on my machine seems created by the missing of this feature:

Thanks for reporting this, it has been fixed in recent commits

Can you confirm me that's the right path?

No, the RgbInvoice structure should not be updated because the feature described in #37 is just related to rgb-lib behavior on how to handle an expired invoice on the receiver side and doesn't require changes to its dependencies. In the _receive method we create an RgbInvoice just to make receive APIs return an invoice string that they can share to receive assets, but the sender doesn't need to know how the receiver decided to handle the expiration of this invoice, therefore this information doesn't need to be encoded in the RgbInvoice structure.

Still sorry for my noobnees, but I am trying to learn more about rgb, but my time is scarse, so I am more going around the code to learn, but I am missing a bit of the bigger scenario of rgb.

No worries, I'm happy that you want to learn more about rgb and I'm happy to help. I hope my answer is clear enough, otherwise don't hesitate to ask more questions.

P.S. please next time just force push the branch of the existing PR instead of creating a new one :)

@arkanoider
Copy link
Author

Hi @arkanoider, we've just released 0.3.0-alpha.1 with support for RGB 0.11 beta 5 so I suggest rebasing this on top of that before proceeding (a lot of things changed).

Sounds clear will do!

Thanks for reporting this, it has been fixed in recent commits

You're welcome, glad that I was right. Thought it was something related to my configuration.

No, the RgbInvoice structure should not be updated because the feature described in #37 is just related to rgb-lib behavior on how to handle an expired invoice on the receiver side and doesn't require changes to its dependencies. In the _receive method we create an RgbInvoice just to make receive APIs return an invoice string that they can share to receive assets, but the sender doesn't need to know how the receiver decided to handle the expiration of this invoice, therefore this information doesn't need to be encoded in the RgbInvoice structure.

Ok! So i can stay inside inside rgb-lib and not touch anything outside! Will proceed in that way in the next days in my spare time.

P.S. please next time just force push the branch of the existing PR instead of creating a new one :)

You're fuckin right! I did some shit and restarted, but forcing a push was the thing to do!

@arkanoider
Copy link
Author

Hi @zoedberg ,

sorry for my silence...:) My fiatjob is killing me in these days, in the next days I will come back on Rgb again!

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.

2 participants