Conversation
WalkthroughThe payroll plugin's Ethereum transaction logic is refactored to use EIP-1559 dynamic fee transactions. Legacy transaction construction and signing are replaced with new methods that generate, encode, and sign unsigned transactions using updated concurrency and helper functions. The transaction proposal and signing completion flows are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Plugin as PayrollPlugin
participant RPC as Ethereum RPC
participant TSS as Threshold Signer
Plugin->>RPC: Concurrently estimate gas, fees, nonce
Plugin->>Plugin: Build unsigned EIP-1559 ERC20 transfer tx
Plugin->>TSS: Propose unsigned tx (hex-encoded) for signing
TSS-->>Plugin: Return TSS signature (R, S, RecoveryID)
Plugin->>Plugin: Append signature to unsigned tx
Plugin->>RPC: Broadcast signed transaction
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)level=warning msg="[runner] Can't run linter goanalysis_metalinter: buildir: failed to load package session: could not load export data: no export data for "github.com/vultisig/go-wrappers/go-dkls/sessions"" 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)plugin/payroll/transaction.go (3)🔇 Additional comments (8)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
plugin/payroll/transaction.go (2)
321-344: Unhandled default branch hides error causeThe
default:case ingenUnsignedTxis empty, then the function falls through to the generic error. Logically unreachable code paths should return a descriptive error inside the switch to avoid accidental success with nil.
404-420: Data-race risk on shared vars
gasLimit,gasTipCap,baseFee,nonceare written inside goroutines without synchronisation. While writes finish before theeg.Wait()return, the Go race-detector still flags unsynchronised writes. Wrap each variable in a struct and assign the pointer inside the goroutine, or compute sequentially—these RPC calls are cheap.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugin/payroll/transaction.go(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
plugin/payroll/transaction.go (1)
158-171: Missing 0x prefix and potential odd-length hex
hex.EncodeToString(tx)yields a bare hex string. Later you feed this value intogcommon.FromHexwhich expects an optional"0x"prefix and even length; if the first byte is < 0x10 the length becomes odd and decoding will panic.
Add the prefix and normalise length before storing / transmitting.- txHex := hex.EncodeToString(tx) +txHex := "0x" + hex.EncodeToString(tx)Likely an incorrect or invalid review comment.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Unit tests to be done in next PRs
Ref: #61
Summary by CodeRabbit
Summary by CodeRabbit