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

Use coreutils/wallet.Event #149

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

chris124567
Copy link
Member

@chris124567 chris124567 commented Dec 9, 2024

Use the standard event type used in our other apps. Unfortunately to avoid duplicating queries this requires some conversions from explorer types to core types. Alternatively, we could just use the enhanced explorer types for the transaction and resolution events which would make things somewhat easier.

Use standard event types from used in other apps, but with explorer type versions of the fields rather than the core versions.

Fix #78

internal/testutil/check.go Outdated Show resolved Hide resolved
persist/sqlite/consensus.go Show resolved Hide resolved
persist/sqlite/events.go Outdated Show resolved Hide resolved
@n8maninger
Copy link
Member

This is pretty rough. I'm leaning towards using the extended types. They are close enough matches that it will probably be fine.

@chris124567
Copy link
Member Author

This is pretty rough. I'm leaning towards using the extended types. They are close enough matches that it will probably be fine.

👍 With you on that. I guess we can still use wallet.Event but the implementing types will be different.

@chris124567
Copy link
Member Author

Started on this but it has me thinking. When we do unconfirmed events, aren't we going to have to convert from the core types into the explorer types? Because we're going to be taking transactions that haven't been indexed yet more or less straight from the txpool. Except we might not have all the info to fill out the "enhanced" fields for the explorer types.

@chris124567 chris124567 force-pushed the christopher/use-wallet-event branch from 37bd37b to 134f5ba Compare December 16, 2024 04:37
@chris124567 chris124567 force-pushed the christopher/use-wallet-event branch from 134f5ba to 9e5777a Compare December 16, 2024 04:56
@chris124567 chris124567 requested review from n8maninger and removed request for n8maninger December 16, 2024 05:01
continue
}

// e.SpentSiafundElements = append(e.SpentSiafundElements, sfe)
Copy link
Member

Choose a reason for hiding this comment

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

commented out line of code


addEvent(types.Hash256(txn.ID()), cs.Index.Height, &e, relevant) // transaction maturity height is the current block height
// ev := EventV2Transaction(txn)
Copy link
Member

Choose a reason for hiding this comment

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

commented out line of code

SiacoinElement: SiacoinOutput{SiacoinElement: element},
Missed: missed,
}, []types.Address{fce.V2FileContract.RenterOutput.Address})
}
Copy link
Member

Choose a reason for hiding this comment

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

We usually don't use scopes like this so I'd avoid them for amore consistent code style across our projects.
e.g. in this case an alternative that provides similar safety with less copy-paste risk would be

		addV2Resolution := func(element types.SiacoinElement) {
			efc := V2FileContract{V2FileContractElement: fce}
			addEvent(types.Hash256(element.ID), element.MaturityHeight, wallet.EventTypeV2ContractResolution, EventV2ContractResolution{
				Resolution: V2FileContractResolution{
					Parent:     efc,
					Type:       typ,
					Resolution: res,
				},
				SiacoinElement: SiacoinOutput{SiacoinElement: element},
				Missed:         missed,
			}, []types.Address{fce.V2FileContract.HostOutput.Address})
		}
		addV2Resolution(sces[types.FileContractID(fce.ID).V2RenterOutputID()])
		addV2Resolution(sces[types.FileContractID(fce.ID).V2HostOutputID()])

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

Use wallet.Event from coreutils
3 participants