XDRill ledger low level helpers example #5501

chowbao · 2024-11-04T15:42:30Z

Example ledgers code for https://github.com/stellar/platform-design-docs/pull/2

sreuland · 2024-11-04T17:29:31Z

exp/xdrill/ledgers/ledgers.go

+	return uint32(l.LedgerHeaderHistoryEntry().Header.LedgerVersion)
+}
+
+func (l Ledgers) GetSorobanFeeWrite1Kb() (int64, bool) {


can probably drop the Get prefix in favor of convention, i.e. any fn here will return a value.

Updated in 146dd52

sreuland · 2024-11-04T17:31:50Z

exp/xdrill/ledgers/ledgers.go

+	return successTxCount, failedTxCount, true
+}
+
+func (l Ledgers) GetOperationCounts() (operationCount, txSetOperationCount int32, ok bool) {


docs on each method should help promote these also, I'm not even sure what is the significance of txSetOperationCount ?

Yeah docs will help. I'll definitely add them in the real PR/implementation.

As for the difference between operationCount, txSetOperationCount is the total operation count (successful + failed) and only successful operation count.

The names are definitely confusing and unclear but I copied these from stellar-etl code that predates me lol
I will definitely rename/refactor these functions in the real PR/implementation

sreuland · 2024-11-04T17:34:05Z

exp/xdrill/ledgers/ledgers.go

+	return operationCount, txSetOperationCount, true
+}
+
+func getTransactionSet(l Ledgers) (transactionProcessing []xdr.TransactionEnvelope) {


would this be where new layer can reference the new non-xdr model instead, i.e. returning a []transactions.Transactions ?

That wasn't the intent of this function. This was just carryover from stellar-etl to count the transactions/operations.

I think you're right though that it would be nice to include a function for users to call to get the non-xdr model instead of manually doing it

sreuland · 2024-11-04T17:42:22Z

exp/xdrill/transform_ledger.go

+	"github.com/stellar/go/xdr"
+)
+
+type LedgerOutput struct {


this appears like state related to a closed ledger, could indicate a bit more like LedgerClosed, etc.

Ah yeah makes sense

Updated in 146dd52

sreuland · 2024-11-04T17:58:53Z

exp/xdrill/transform_ledger.go

+	TotalByteSizeOfBucketList  uint64    `json:"total_byte_size_of_bucket_list"`
+}
+
+func TransformLedger(lcm xdr.LedgerCloseMeta) (LedgerOutput, error) {


will this pattern scale well for the sdk, i.e. what will the next fn be named that transforms to a different model, the pattern implies something like TransformLedgerTo<DerivedModel>() <DerivedModel> which is probably fine, this fn is specifically TransformLedgerToOutput correct?

this fn is specifically TransformLedgerToOutput correct?

Yes

I didn't think of the pattern for how to name custom processors. This is just how we define/call it in stellar-etl and was mostly used to serve as one possible way to call the low level helper functions.

It makes sense to standardize the name/pattern though. I think it would be hard though.
I think there's currently the idea from like

OffersProcessor() with HasOffer and DeriveOffer

stellar-etl has an input/*.go functions to read the ledgers --> transform/*.go like the above TransformLedger to process the data

TransformLedgerTo<DerivedModel>() <DerivedModel> TransformLedgerToOutput as noted above

And I think there'd then be another layer depending on what kind of output/data model the user wants for their application. Like a user could rightfully/wrongfully do:

func ProcessLedgerMetadata(lcm xdr.LedgerCloseMeta) { TransformLedgerToOutput() TransformOfferToOutput() if ingest.HasPayment(lcm) { ingest.DerivePayment(lcm) } }

This might be worth bringing back to the design doc or in the team meeting to discuss

yes, good suggestion on opening up to further design discussion, another option to consider may be to remove the transformer package to avoid grey area where app domain opinions inevitably start leaking in as the derived model, refactor this transform as a more factual attribute retrieval like ledgers.Ledgers.Closed() ClosedOutput, ideally this new sdk of low level helpers should give developers enough tooling that they can quickly write their own lcm-to-model transforms, rather than the sdk codifying transforms?

ideally this new sdk of low level helpers should give developers enough tooling that they can quickly write their own lcm-to-model transforms, rather than the sdk codifying transforms?

Yes 100%

sreuland · 2024-11-04T18:41:49Z

exp/xdrill/ledgers/ledgers.go

+	"github.com/stellar/go/xdr"
+)
+
+type Ledgers struct {


should this be singular,Ledger, all the receiver methods imply such.

Ah yeah that's a good catch. I'll make the packages, structs, and functions all singular instead of plural

Updated in 3b90190

socket-security · 2024-11-08T06:16:52Z

New dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
golang/github.com/dgryski/[email protected]	None	`0`	84.6 kB

View full report↗︎

chowbao · 2024-11-08T06:20:09Z

exp/xdrill/transform_ledger_xdr.go

Example using helper functions only in xdr. I think for the LedgerClosed case this works really well

Type casting should be removed and just added to the functions themselves if possible

chowbao · 2024-11-08T06:24:10Z

ingest/ledger_operation.go

It seemed to make sense to create a new LedgerOperation type similar to the operationWrapper in Horizon and stellar-etl.

I'm not sure how some of these functions would exist in the xdr package such as getting OperationID which uses the LedgerCloseMeta, Transaction, and the OperationIndex.

Some helper functions could be added to xdr/operations.go but it feels like a bad user experience switching between the two when all the helper functions can be defined in one spot (ingest/ledger_operation.go) instead

chowbao · 2024-11-08T06:37:50Z

xdr/account_entry.go

Didn't look into other ledgerEntries but it seems like adding helper functions to the xdr seems good. The only issue I found was that for a ledgerEntry like AccountEntry there are existing functions and fields that are of xdr.Int64 or xdr.Uint32 which would force the user to type cast in their application. It's just a small annoying thing but I wonder if it would be worthwhile to figure out a way to remove the casting while still using a reasonable function name

Another thing with ledgerEntries that I found was it's awkward to get to a specific entry type like AccountEntry before calling these functions. I couldn't figure out a way to allow the user to use one of Changes or LedgerEntry themselves to get specific data because some EntryTypes have similarly named fields like Balance. Is it possible to make LedgerEntry functions smart enough to return the correct field from the correct entryType?

// This could work but I feel like it might be confusing to users where the balance actually came from if the user doesn't realize they need to look at the LedgerEntry.Type // Maybe this is fine because the user really should be checking LedgerEntry.Type anyways func (l LedgerEntry) Balance() float64 { switch l.Type { case LedgerEntryTypeAccount: return l.Account.Balance case LedgerEntryTypeTrustline: return l.TrustLine.Balance } }

chowbao · 2024-11-08T06:39:21Z

xdr/ledger_close_meta.go

Added helper functions to LCM

XDRill ledger low level helpers example

d222d6e

sreuland reviewed Nov 4, 2024

View reviewed changes

Address comments

146dd52

sreuland reviewed Nov 4, 2024

View reviewed changes

chowbao added 2 commits November 4, 2024 13:57

Make names singular instead of plural

3b90190

try helper functions in ingest and xdr

6352001

chowbao commented Nov 8, 2024

View reviewed changes

xdr/ledger_close_meta.go

Copy link

Contributor Author

chowbao Nov 8, 2024

Choose a reason for hiding this comment

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

Added helper functions to LCM

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

XDRill ledger low level helpers example #5501

XDRill ledger low level helpers example #5501

chowbao commented Nov 4, 2024

sreuland Nov 4, 2024

chowbao Nov 4, 2024

sreuland Nov 4, 2024

chowbao Nov 4, 2024

sreuland Nov 4, 2024 •

edited

Loading

chowbao Nov 4, 2024

sreuland Nov 4, 2024

chowbao Nov 4, 2024

chowbao Nov 4, 2024

sreuland Nov 4, 2024

chowbao Nov 4, 2024

sreuland Nov 4, 2024

chowbao Nov 4, 2024

sreuland Nov 4, 2024

chowbao Nov 4, 2024

chowbao Nov 4, 2024

socket-security bot commented Nov 8, 2024

chowbao Nov 8, 2024

chowbao Nov 8, 2024

chowbao Nov 8, 2024 •

edited

Loading

chowbao Nov 8, 2024

XDRill ledger low level helpers example #5501

Are you sure you want to change the base?

XDRill ledger low level helpers example #5501

Conversation

chowbao commented Nov 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sreuland Nov 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

socket-security bot commented Nov 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chowbao Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sreuland Nov 4, 2024 •

edited

Loading

chowbao Nov 8, 2024 •

edited

Loading