feat: expose self-trade attributes in sharedlib and wasm bindings#76
feat: expose self-trade attributes in sharedlib and wasm bindings#76mihaimarcu2004 wants to merge 1 commit into
Conversation
Co-Authored-By: mihai <mihai2004marcu@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| if skipNonce == 1 { | ||
| attr.SkipNonce = &skipNonce | ||
| } | ||
| if selfTradeBehaviorMode != txtypes.SelfTradeBehaviorExpireMaker { | ||
| attr.SelfTradeBehaviorMode = &selfTradeBehaviorMode | ||
| } | ||
| if selfTradeEqualityMode != txtypes.SelfTradeEqualityAccountIndex { | ||
| attr.SelfTradeEqualityMode = &selfTradeEqualityMode | ||
| } | ||
| return &attr |
There was a problem hiding this comment.
🔴 Self-trade attributes unusable due to nil-valued integrator entries exceeding NbAttributesPerTx limit
Both CreateIntegratorTxAttributes (sharedlib) and integratorTxAttributes (wasm) unconditionally set IntegratorAccountIndex, IntegratorTakerFee, and IntegratorMakerFee pointers — even when their values are 0 (the nil value). ConstructL2TxAttributes (types/tx_request.go:193-201) then adds these to the L2TxAttributes map since the pointers are non-nil. The Validate() method at types/txtypes/tx_attributes.go:129 checks len(attr) > NbAttributesPerTx where NbAttributesPerTx=4 (types/txtypes/constants.go:198), counting ALL entries including nil-valued ones.
This means using any non-default self-trade mode with skipNonce produces 5 entries (3 nil-valued integrator + 1 selfTrade + 1 skipNonce) → ErrTooManyAttributes. Using BOTH non-default self-trade modes produces 5 entries even without skipNonce (3 nil-valued integrator + 2 selfTrade) → also fails. The feature is effectively broken for these common use cases.
Example failing scenario trace
Calling SignCreateOrder with integratorAccountIndex=0, integratorTakerFee=0, integratorMakerFee=0, selfTradeBehaviorMode=1, selfTradeEqualityMode=0, skipNonce=1:
CreateIntegratorTxAttributessets: IntegratorAccountIndex=&0, IntegratorTakerFee=&0, IntegratorMakerFee=&0, SkipNonce=&1, SelfTradeBehaviorMode=&1ConstructL2TxAttributesadds 5 map entries (all pointers non-nil)Validate()seeslen(attr)=5 > 4→ returnsErrTooManyAttributes
The nil-valued integrator entries don't affect the hash (skipped in getNormalizedTypes) and IsEmpty() considers them empty, but they still count toward the 4-attribute limit.
(Refers to lines 124-138)
Prompt for agents
The CreateIntegratorTxAttributes function in sharedlib/main.go (and the equivalent integratorTxAttributes in wasm/main.go) unconditionally sets IntegratorAccountIndex, IntegratorTakerFee, and IntegratorMakerFee pointers even when their values equal the nil sentinel values (all 0). This wastes attribute slots in the L2TxAttributes map, which has a hard limit of NbAttributesPerTx=4 entries.
The fix is to only set these pointers when the values differ from the nil values. Both functions need the same change:
1. sharedlib/main.go CreateIntegratorTxAttributes (around line 124-138): Instead of unconditionally setting attr.IntegratorAccountIndex, attr.IntegratorTakerFee, and attr.IntegratorMakerFee, guard each with a nil-value check, similar to the self-trade and skipNonce fields. The nil values are: NilIntegratorIndex=0 (txtypes constant), NilIntegratorTakerFee=0, NilIntegratorMakerFee=0.
2. wasm/main.go integratorTxAttributes (around line 38-53): Same change — guard the integrator fields with nil-value checks instead of unconditionally including them in the struct initializer.
This is safe because absent entries and nil-valued entries behave identically: GetValueOrDefault returns the nil value for missing keys, and getNormalizedTypes and IsEmpty both skip nil-valued entries.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Confirmed and fixed. Reproduced exactly as described: CreateIntegratorTxAttributes (sharedlib) / integratorTxAttributes (wasm) set the 3 integrator pointers unconditionally, so an order with integrator values 0 + a non-default self-trade mode + skipNonce produced 5 map entries ({1:0, 2:0, 3:0, 4:1, 6:1}) → ErrTooManyAttributes (limit NbAttributesPerTx=4).
Fix: only set the integrator pointers when the value differs from its nil sentinel (NilIntegratorIndex/NilIntegratorTakerFee/NilIntegratorMakerFee, all 0), matching how skipNonce and the self-trade modes are already handled.
This is signature-preserving: getNormalizedTypes() already skips nil-valued entries, so the attribute Hash() is byte-identical with or without the 0-valued integrator keys. Verified with a unit check — old map (with nil integrator entries) fails Validate(), new map passes, and old.Hash() == new.Hash(). WASM SignCreateOrder now emits L2TxAttributes":{"4":1} instead of {"1":0,"2":0,"3":0,"4":1}.
Note: PR #76 was closed at the requester's instruction — the fix landed directly on self-trade as commit 3718eab (alongside the original d09f1e0).
Summary
The
self-tradebranch (#75) addedSelfTradeBehaviorMode,SelfTradeEqualityMode, andCancelAllMarketIndextotypes.L2TxAttributes, but only at thetypes/txtypeslayer — the C-FFI (sharedlib/main.go) and WASM (wasm/main.go) entrypoints had no way to set them. This PR wires those three attributes through both bindings via the existing attribute builders. Targets theself-tradebranch since it depends on those type changes.What's exposed where:
SignCreateOrder/SignCreateGroupedOrders/SignModifyOrder→ newselfTradeBehaviorMode,selfTradeEqualityModeparamsSignCancelAllOrders→ newcancelAllMarketIndexparamBuilders only set a pointer when the value differs from its nil/default (
SelfTradeBehaviorExpireMaker=0,SelfTradeEqualityAccountIndex=0,NilMarketIndex=255), so passing defaults produces byte-identical signatures to before (verified: WASMSignCreateOrderstill emitsL2TxAttributeswithout keys 6/7).New args are inserted after the integrator-fee params and before
skipNonce, keepingapiKeyIndex/accountIndexlast (WASMgetClientreads them from the tail). WASM arg-count checks and "expects N args" messages bumped accordingly.Example call sites (required to keep CI green)
CI compiles and runs the C++/Java/Rust/WASM examples, which call these exported functions, so their call sites were mechanically updated to pass the new defaults (
0/0/255) — no logic changes. This is the only reason the PR touchesexamples/.Verification
go build -buildmode=c-shared(sharedlib) + generated header signatures confirmedgo build GOOS=js GOARCH=wasm(wasm) +node examples/wasm/test_wasm.mjs→ all assertions passcargo build --release(rust example) + g++ compile of cpp examplegofmt,go vet,go test ./...cleanmvnnot available locally; JNA interface updated to match the generated header)Link to Devin session: https://app.devin.ai/sessions/08b2387465664e789286a14d3078a1a4
Requested by: @mihaimarcu2004