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

Beginning of payment history database #21

Merged
merged 2 commits into from
May 15, 2024
Merged

Beginning of payment history database #21

merged 2 commits into from
May 15, 2024

Conversation

benthecarman
Copy link
Contributor

Want some ACKs on the db schema before I go further. Even added some unit tests 👀

@benthecarman benthecarman requested a review from futurepaul May 14, 2024 23:06
Comment on lines 27 to 65
CREATE TABLE on_chain
(
operation_id TEXT PRIMARY KEY NOT NULL,
fedimint_id TEXT NOT NULL REFERENCES fedimint (id),
address TEXT NOT NULL,
amount_sats BIGINT NOT NULL,
inbound INTEGER NOT NULL,
txid TEXT,
status INTEGER NOT NULL,
created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP
);
Copy link
Contributor

@TonyGiorgio TonyGiorgio May 15, 2024

Choose a reason for hiding this comment

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

It's worth considering splitting on chain deposits and on chain withdraws. Same with lightning too. It's been a pain in the ass trying to conform them both in mutiny.

You have a few things in both that are marked NOT NULL and those would be different based on whether it's a deposit or a withdraw.

@benthecarman
Copy link
Contributor Author

ready for round 2

@benthecarman benthecarman requested a review from TonyGiorgio May 15, 2024 02:19
@TonyGiorgio
Copy link
Contributor

I think the structure is sound, should merge asap even if it doesn't hook in throughout yet

@benthecarman benthecarman merged commit 1d5a298 into master May 15, 2024
2 checks passed
@benthecarman benthecarman deleted the history branch May 15, 2024 03:48
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