Page MenuHomePhabricator

[ecash-agora] Partial Agora offers: Add approximation logic
ClosedPublic

Authored by tobias_ruck on Sep 7 2024, 16:16.

Details

Summary

Add AgoraPartial and AgoraPartialParams for offers of fungible tokens, and add (only) the script param approximation logic.

This is rather complex, and is explained in the comments. The corresponding Script can be seen in D16644, which will be added in later diffs.

Test Plan

npm run integration-tests

Diff Detail

Repository
rABC Bitcoin ABC
Branch
ecash-agora-partial-approx
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 30218
Build 59963: Build Diffecash-agora-integration-tests
Build 59962: arc lint + arc unit

Event Timeline

move approx test into src

add many more test cases, improve approximation in some cases

Fabien added inline comments.
modules/ecash-agora/src/partial.ts
222

Nit: this is probably quite obvious but scriptUnsignedIntegerBits is more accurate (or just scriptUintBits)

235

Should it be 0x7fffffffffffffffn to avoid negative values ?

243

same question

251

Dito, maxScriptUint

253
274

This comment confused me, I thought you were referring to the max supply somehow with this 21M value :) Please use 2^31 to clarify

modules/ecash-agora/src/partial.ts
222

hmm, I get your point, but I don't really like either of those names, the "unsigned" seems misleading.

we could also keep scriptIntegerBits, but then have it be 32 (and 64) respectively, and simply subtract the sign bit

or maybe better numScriptIntegerBits

235

No, this is what the SLP spec allows, so we should follow it.

We could go higher (and truncate more), but that ofc woudn't make sense since these numbers couldn't be represented in SLP

243

same here, this is simply the max number for ALP values

251

here it's literally the max script integer, so I think this is fine

274

good point :)

modules/ecash-agora/src/partial.ts
222

I like the idea of setting to 32 and apply -1 on the sign bit

404–406

This works and I might just be too used to bit masks, but I prefer this version. Your call.

428–429

Same, feel free to ignore if you disagree

modules/ecash-agora/src/partial.ts
37
118

what is the most inaccurate prices can be in this current implementation?

modules/ecash-agora/src/partial.ts
118

There's some degenerate cases, especially when accepting just 1 token of something

The tests go through different scenarios, some are exactly accurate, some are quite a bit off. For example, "AgoraPartial.approximateParams 2p63-1, small price" is off by 190x (15258 nsats/token instead of 80) for accepting 0x10000000000 tokens, but that's an extreme example.

You can see the others are mostly within 5% for the smallest accept amount, and if you even accept like 5 or 10 accuracy goes up quickly.

We can refine the estimation later over time, without changing the script, so only makers would need to upgrade to get the better precision.

404–406

I tried this version but it turned out more complicated IMO.

Also while this may be very normal bit arithmetic for C++ devs, it's more novel for TS devs, so I try to use simpler operations (like shifting right then left is IMO easier to understand than shifting and then subtracting 1)

428–429

Yes, again I'm just not convinced this is more obvious to a TS dev this way

improve comments, use 32 for bits, address other nits

add test to build, fix test

This revision is now accepted and ready to land.Sep 11 2024, 08:50