Add Gasless System #1
Conversation
…nce gas fee checks - Provide EVM with gas to process gasless txns - Update preCheck to ignore 1559 fee check logic for gasless txns
- Implement logic to retrieve and merge transactions that satisfy the base fee filter with those having a GasFeeCap of 0. - Prioritize transactions from the base fee set while ensuring zero-fee transactions are included.
- Modify the newTxWithMinerFee function to allow zero-fee transactions by setting baseFee to nil when GasFeeCap is zero. - Ensure that the ordering of pending transactions in the txpool accounts for both fee-paying and zero-fee transactions.
- Introduce a TODO comment in ValidateTransactionWithState to handle cases where tx.GasTipCap() is zero. - Outline the need to call the gasStation precompile to validate gasless transactions, ensuring proper checks for the transaction's recipient and data.
- Introduce a TODO comment in preCheck to address scenarios where tx.GasTipCap() is zero. - Highlight the need to call the gasStation precompile to potentially reduce quota for gasless transactions.
- Implement isGaslessTx method across various transaction types (DepositTx, AccessListTx, BlobTx, DynamicFeeTx, LegacyTx, SetCodeTx) to determine if a transaction is gasless. - Enhance transaction interfaces to include gasless transaction checks, facilitating better handling of zero-fee transactions.
- Update ValidateTransaction to enforce minimum gas price requirements for non-gasless transactions while allowing gasless transactions to bypass this check. - Modify ValidateTransactionWithState to skip balance checks for gasless transactions, with a TODO to implement validation against the gasStation precompile for gasless transaction legitimacy.
…force min tips unless gasless - This allows us to enfore minTip/gasPrice for the miner while only allowing gaslessTx
…date transaction handling - Introduce IsGaslessTx field in the Message struct to indicate gasless transactions. - Update TransactionToMessage function to include the new IsGaslessTx field. - Modify preCheck function to skip gas fee checks for gasless transactions, enhancing transaction validation logic.
- Clean up code by removing an extraneous blank line in the fillTransactions function, improving readability.
- Added initialization for GaslessRegistryAddress in the DeveloperGenesisBlock function, setting its Nonce, Code, and an empty Storage map. This enhances the genesis block setup for gasless transactions.
- Introduced `GasStationStorageSlots` struct to manage storage slot hashes for gasless transactions. - Added `calculateGasStationSlots` function to compute storage slot hashes for registered contracts. - Implemented `validateGaslessTx` function to validate gasless transactions, checking registration, activity status, credits, and whitelist status. - Updated `ValidateTransactionWithState` to call `validateGaslessTx` for gasless transactions, enhancing overall transaction validation.
…ess transactions - Added logic to call the gas station's chargeCredits method when processing gasless transactions. - Updated innerExecute function to handle gasless transaction credit charges, enhancing transaction execution flow.
…it usage tracking - Added `PendingCreditUsage` callback to `ValidationOptionsWithState` for tracking credits used by pending transactions. - Updated `validateGaslessTx` to consider pending credit usage when validating available credits for gasless transactions. - Implemented methods in `LegacyPool` to manage pending credit usage, including incrementing and decrementing usage as transactions are added or removed.
- Implemented a check in the RollupCostData method to return an empty RollupCostData for gasless transactions, improving efficiency in cost calculations.
- Adjusted gas handling in the buyGas method to ensure that gas is correctly allocated and subtracted for gasless transactions, improving transaction processing accuracy.
- Increased gas remaining for gasless transactions by an additional 10,000 units in the buyGas method, ensuring more accurate gas management during transaction processing.
…lds method - Updated the DeriveFields method to account for gasless transactions alongside deposit transactions, ensuring accurate gas price assignment for gasless transactions
- Adjusted initial gas allocation in the buyGas method by adding 5000 units to cover the gasStation call, while maintaining the gas remaining equal to the gas limit. This change enhances the accuracy of gas management for gasless transactions.
- Modified the GasStationCode variable to reflect updated bytecode for gasless transaction handling. This change ensures compatibility with the latest gas station logic and improves the overall functionality of gasless transactions within the protocol.
- Added a new map to track pending credit usage for each address in the LegacyPool. This enhancement improves the management of credit usage, ensuring accurate tracking for transactions.
- Introduced a new GasStation module that includes the calculation of storage slots and validation for gasless transactions. This implementation enhances the management of gasless transactions by ensuring proper credit checks and whitelist validations, improving overall transaction integrity and functionality.
…ation - Added validation for gasless transaction requirements in the preCheck method to ensure proper checks before processing. - Updated innerExecute to calculate and deduct credits based on the final gas used, improving accuracy in credit management. - Implemented ABI encoding for event logging of gasless transactions, enhancing the clarity and integrity of transaction records.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
core/state_transition.go (1)
696-704:⚠️ Potential issueSkip
returnGas()for gasless transactions
After the refund is applied the sender currently receives ETH for unused gas
despite never having paid for it. Move the crediting logic behind a flag.- st.returnGas() + if !msg.IsGaslessTx { + st.returnGas() + } else { + // still free the gas pool + st.gp.AddGas(st.gasRemaining) + }This eliminates unintended ETH minting while keeping the block-level gas
accounting correct.
🧹 Nitpick comments (3)
core/state_transition.go (2)
25-32: Import grouping – moveabinext to othergo-ethereum/accountsimports
Tiny nit: keep related imports together to preserve logical grouping (std lib / external / internal).
666-676: Ignoreabi.NewTypeerrors & potentialuint64→int64overflow
- Always check the error from
abi.NewType, otherwise a malformed type string
will panic later.int64(st.gasUsed())will overflow if block gas limit ever exceedsMaxInt64
(the theoretical limit keeps growing). Usebig.NewInt(0).SetUint64(...).- addressType, _ := abi.NewType("address", "", nil) - uint256Type, _ := abi.NewType("uint256", "", nil) + addressType, err := abi.NewType("address", "", nil) + if err != nil { panic(err) } + uint256Type, err := abi.NewType("uint256", "", nil) + if err != nil { panic(err) } ... - data, err := arguments.Pack(st.msg.From, big.NewInt(int64(st.gasUsed()))) + gasUsedBig := new(big.Int).SetUint64(st.gasUsed()) + data, err := arguments.Pack(st.msg.From, gasUsedBig)core/gas_station.go (1)
25-60:CalculateGasStationSlots– pointer mutation & constant reuse
gasStationStorageLocation.Big()returns the samebig.Intbacking array
each call. Adding to it mutates the constant. Copy before mutation:- contractsMapSlot := new(big.Int).Add(gasStationStorageLocation.Big(), big.NewInt(1)) + contractsMapSlot := new(big.Int).Add(new(big.Int).Set(gasStationStorageLocation.Big()), big.NewInt(1))
- Consider caching the event-signature hash / storage location as global
var
to avoid repeated allocations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
core/gas_station.go(1 hunks)core/state_transition.go(5 hunks)core/txpool/validation.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- core/txpool/validation.go
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build Image & Push to ECR
🔇 Additional comments (4)
core/state_transition.go (2)
175-176: Field addition looks good
IsGaslessTxis clearly documented and scoped.
200-201: Propagation ofIsGaslessTxflag correctly wired
The new flag is passed throughTransactionToMessage.core/gas_station.go (2)
13-20: Struct definition is clear and matches the storage layout
No issues spotted.
62-124: Validation gaps & out-of-bounds safety
- Missing fee-field validation: caller can set
GasPrice>0and later receive a
refund (see earlier comment).storageBaseSlot[30]/ … access a fixed index; add a length guard to avoid
panic ifGetStateever returns a shorter slice (mock back-ends etc.).- When whitelist is enabled you do one storage read per tx. The white-list
path is hot; cache the mapping base slot hash from the earlier calculation
instead of recomputing.Minimal defensive fix:
if len(storageBaseSlot) != 32 { return nil, nil, nil, fmt.Errorf("unexpected storage slot length %d", len(storageBaseSlot)) }
- Introduced a StateReader interface to decouple gasless transaction validation from the state implementation, enhancing flexibility and testability. - Updated ValidateGaslessTx function to accept StateReader instead of a specific state implementation, allowing for broader compatibility with different state management systems.
- Updated innerExecute to use a big.Int for gasUsed, improving precision in ABI encoding of transaction data. This change enhances the accuracy of gas management in state transitions.
- Introduced a variable for the CreditsUsed event signature to improve code readability and maintainability. - Updated innerExecute method to utilize the new CreditsUsedEventSignature, enhancing consistency in event logging for gasless transactions.
- Added pre-built ABI arguments for the CreditsUsed event to streamline event logging in gasless transactions. - Updated innerExecute method to utilize the new CreditsUsedEventArgs for improved clarity and consistency in ABI encoding.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
core/gas_station.go (2)
49-50: Document the source of the hardcoded storage location hash.The ERC-7201 storage location constant should include a reference to how this value was computed or where it originates from to ensure maintainability and verification.
// ERC-7201 storage location for GasStationStorage -// bytes32 private constant GasStationStorageLocation = 0xc2eaf2cedf9e23687c6eb7c4717aa3eacbd015cc86eaad3f51aae2d3c955db00; +// bytes32 private constant GasStationStorageLocation = keccak256(abi.encode(uint256(keccak256("gasstation.storage")) - 1)) & ~bytes32(uint256(0xff)); +// Computed value: 0xc2eaf2cedf9e23687c6eb7c4717aa3eacbd015cc86eaad3f51aae2d3c955db00 gasStationStorageLocation := common.HexToHash("0xc2eaf2cedf9e23687c6eb7c4717aa3eacbd015cc86eaad3f51aae2d3c955db00")
113-115: Consider adding overflow protection for credit arithmetic.While unlikely with reasonable gas limits, the credit comparison should be protected against potential overflow scenarios in production environments.
// Check if contract has enough available credits to cover the cost of the tx + if txRequiredCreditsBig.Cmp(big.NewInt(0)) <= 0 { + return nil, nil, nil, fmt.Errorf("invalid gas limit for gasless transaction") + } if availableCreditsBig.Cmp(txRequiredCreditsBig) < 0 { return nil, nil, nil, fmt.Errorf("gasless transaction has insufficient credits: have %v, need %v", availableCreditsBig, txRequiredCreditsBig) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
core/gas_station.go(1 hunks)core/state_transition.go(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- core/state_transition.go
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build Image & Push to ECR
🔇 Additional comments (3)
core/gas_station.go (3)
45-80: LGTM: Storage slot calculation logic is correct.The implementation correctly follows Ethereum's storage layout conventions for mappings and struct offsets. The ERC-7201 pattern usage and keccak256 hashing for mapping slots is properly implemented.
123-140: LGTM: Whitelist validation logic is well-implemented.The nested mapping slot calculation and whitelist validation follows the correct Ethereum storage patterns. The conditional whitelist enforcement provides good flexibility.
91-96:❓ Verification inconclusive
Verify storage layout assumptions for packed struct fields.
The code assumes specific byte positions for
registered(byte 31) andactive(byte 30) flags. This is critical for correctness and should be verified against the actual Solidity struct layout.
🏁 Script executed:
#!/bin/bash # Verify the GasStation contract storage layout matches the Go implementation assumptions echo "Searching for GasStation contract storage layout..." rg -A 10 -B 5 "struct.*GasStation|bool registered|bool active" --type solLength of output: 216
🏁 Script executed:
#!/bin/bash # Search for the GasStation struct definition in Solidity files echo "Locating GasStation struct definition..." rg -A10 -B5 "struct\s+GasStation" -g '*.sol' # Search for the bool fields `registered` and `active` in Solidity files echo "Verifying presence and order of bool fields..." rg -n "bool\s+registered" -g '*.sol' rg -n "bool\s+active" -g '*.sol'Length of output: 314
Verify Solidity
GasStationstruct byte offsetsI wasn’t able to locate the
GasStationstruct definition in the repo to confirm thatregisteredandactivemap to bytes 31 and 30 of the base storage slot. Please manually verify that your Solidity layout matches these Go assumptions:• File: core/gas_station.go
Lines 91–96:storageBaseSlot := sdb.GetState(params.GasStationAddress, gasStationStorageSlots.StructBaseSlotHash) // Extract the registered and active bytes from the storage slot isRegistered := storageBaseSlot[31] == 0x01 isActive := storageBaseSlot[30] == 0x01• In your Solidity contract ensure:
- The
GasStationstruct is in the same slot and order as assumed.bool registeredis the last byte (offset 31) of the slot.bool activeis the second-to-last byte (offset 30).If the struct or its packing changes, adjust the byte indices or use a safer unpacking helper.
…ansactions - Enhanced the ValidateGaslessTx function to include a check for a non-zero gas limit, ensuring that gasless transactions are properly validated before processing. This change improves the robustness of transaction handling and prevents potential errors related to gas limits.
- Refactored the innerExecute method to streamline the logging of CreditsUsed events by removing fallback manual encoding. The method now directly adds logs when ABI encoding is successful, improving code clarity and reducing redundancy.
- Replaced the hardcoded gas station storage location with a computed value using keccak256 for improved clarity and maintainability. This change enhances the flexibility of the gas station implementation by allowing for dynamic calculation of the storage location.
…ransactions - Introduced a single-use mode in the gas station implementation, allowing for enhanced control over gasless transactions. - Added logic to track whether a user has utilized gasless transactions, preventing multiple uses if single-use mode is enabled. This change improves the integrity and security of gasless transaction handling.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
core/gas_station.go (2)
47-92: Storage slot calculation logic is correct but could benefit from helper function.The function correctly implements EVM storage layout patterns and ERC-7201 storage location computation. The logic is sound and follows Ethereum storage conventions.
Consider extracting the nested mapping slot calculation into a helper function to reduce code duplication:
+// calculateNestedMappingSlot computes the storage slot hash for a nested mapping +func calculateNestedMappingSlot(key common.Address, baseSlot common.Hash) common.Hash { + keyPadded := common.LeftPadBytes(key.Bytes(), 32) + mapBaseSlotPadded := common.LeftPadBytes(baseSlot.Bytes(), 32) + combined := append(keyPadded, mapBaseSlotPadded...) + return crypto.Keccak256Hash(combined) +}This helper could then be used in the validation function to eliminate duplicate mapping slot calculation logic.
94-181: Comprehensive validation logic with previous feedback addressed.The validation function is well-implemented and addresses all previous review feedback. The gas limit validation has been properly added. The logic thoroughly checks all required conditions for gasless transactions.
The nested mapping slot calculations on lines 141-144 and 164-167 are duplicated. Consider using the helper function suggested above:
if isWhitelistEnabled { - // Calculate slot for the specific user in the nested whitelist map - userKeyPadded := common.LeftPadBytes(from.Bytes(), 32) - mapBaseSlotPadded := common.LeftPadBytes(gasStationStorageSlots.NestedWhitelistMapBaseSlotHash.Bytes(), 32) - userCombined := append(userKeyPadded, mapBaseSlotPadded...) - userWhitelistSlotHash := crypto.Keccak256Hash(userCombined) + userWhitelistSlotHash := calculateNestedMappingSlot(from, gasStationStorageSlots.NestedWhitelistMapBaseSlotHash) // ... later in the function ... if isSingleUseEnabled { - // Calculate slot for the specific user in the nested usedAddresses map - userKeyPadded := common.LeftPadBytes(from.Bytes(), 32) - mapBaseSlotPadded := common.LeftPadBytes(gasStationStorageSlots.UsedAddressesMapBaseSlotHash.Bytes(), 32) - userCombined := append(userKeyPadded, mapBaseSlotPadded...) - userUsedSlotHash := crypto.Keccak256Hash(userCombined) + userUsedSlotHash := calculateNestedMappingSlot(from, gasStationStorageSlots.UsedAddressesMapBaseSlotHash)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
core/gas_station.go(1 hunks)core/state_transition.go(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- core/state_transition.go
🧰 Additional context used
🧠 Learnings (1)
core/gas_station.go (1)
Learnt from: Sledro
PR: lightlink-network/ll-geth#1
File: core/gas_station.go:14-24
Timestamp: 2025-06-06T13:53:51.113Z
Learning: In Ethereum event encoding, ABI Arguments should only contain non-indexed parameters. Indexed parameters are handled separately in the Topics array. The event signature includes all parameters (indexed and non-indexed), but ABI Arguments only defines the types for non-indexed parameters that get ABI-encoded in the event data field.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build Image & Push to ECR
🔇 Additional comments (4)
core/gas_station.go (4)
1-11: LGTM: Clean package structure and appropriate imports.The package declaration and imports are well-organized and include all necessary dependencies for the gasless transaction functionality.
13-25: Event signature and ABI definitions are correctly implemented.Based on the retrieved learnings, the implementation correctly follows Ethereum event encoding standards where ABI Arguments contain only non-indexed parameters while indexed parameters are handled separately in the Topics array.
27-31: Well-designed interface for state abstraction.The
StateReaderinterface provides good abstraction, allowing the validation logic to work with any state implementation that can read storage.
33-41: Clear and comprehensive storage slot structure.The
GasStationStorageSlotsstruct effectively organizes all the storage slot hashes needed for gasless transaction validation.
- Simplified the calculation of storage slots for user whitelists and used addresses by introducing a new function, calculateNestedMappingSlot. This change enhances code clarity and maintainability by reducing redundancy in slot hash calculations.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
core/gas_station.go (1)
94-175: Consider refactoring ValidateGaslessTx for better maintainability.The function correctly implements all validation logic and security checks, but it's quite long and handles multiple concerns. Consider breaking it down into smaller, focused helper functions.
For example, you could extract helper functions like:
func validateContractRegistration(sdb StateReader, slots *GasStationStorageSlots) error { // Lines 106-119 } func validateCredits(sdb StateReader, slots *GasStationStorageSlots, gasLimit uint64) (*big.Int, *big.Int, error) { // Lines 121-131 } func validateWhitelist(sdb StateReader, slots *GasStationStorageSlots, from common.Address) error { // Lines 133-153 } func validateSingleUse(sdb StateReader, slots *GasStationStorageSlots, from common.Address) error { // Lines 155-172 }This would improve readability and make each validation concern easier to test and maintain independently.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
core/gas_station.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
core/gas_station.go (1)
Learnt from: Sledro
PR: lightlink-network/ll-geth#1
File: core/gas_station.go:14-24
Timestamp: 2025-06-06T13:53:51.113Z
Learning: In Ethereum event encoding, ABI Arguments should only contain non-indexed parameters. Indexed parameters are handled separately in the Topics array. The event signature includes all parameters (indexed and non-indexed), but ABI Arguments only defines the types for non-indexed parameters that get ABI-encoded in the event data field.
🧬 Code Graph Analysis (1)
core/gas_station.go (4)
crypto/crypto.go (1)
Keccak256Hash(97-104)accounts/abi/type.go (1)
NewType(73-231)common/types.go (2)
HexToHash(72-72)BigToHash(68-68)params/protocol_params.go (1)
GasStationAddress(237-237)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build Image & Push to ECR
🔇 Additional comments (6)
core/gas_station.go (6)
1-11: LGTM!Package declaration and imports are appropriate for the gasless transaction validation functionality being implemented.
13-25: Event signature and ABI arguments are correctly implemented.Based on the retrieved learnings, this implementation correctly follows Ethereum standards where ABI Arguments only contain non-indexed parameters while indexed parameters are handled separately in the Topics array. The event signature
CreditsUsed(address,address,uint256)has 3 parameters butCreditsUsedEventArgscorrectly defines only 2 non-indexed parameters (callerandgasUsed).
27-31: StateReader interface is well-designed.The minimal interface design allows for flexible testing and abstraction over different state implementations while providing exactly what's needed for storage access.
33-41: GasStationStorageSlots struct is well-structured.The field names are descriptive and clearly indicate their purpose in the storage layout. The struct provides a clean way to organize all the computed storage slot hashes.
43-92: CalculateGasStationSlots function correctly implements storage slot calculations.The function properly implements ERC-7201 storage location patterns and follows Ethereum storage layout rules. The documented struct layout with offsets is helpful for understanding the storage organization.
177-183: Helper function is correctly implemented.The
calculateNestedMappingSlotfunction properly computes storage slot hashes for nested mappings following Ethereum storage layout rules.
…new keccak256 value - Replaced the previous hardcoded gas station storage location with a new computed value using keccak256 for improved clarity and maintainability. This change enhances the flexibility of the gas station implementation by allowing for dynamic calculation of the storage location.
…dard tx - Added pre-validation for gasless transactions in the ApplyMessage function to ensure proper handling before gas metering. - Introduced a new method, handleGaslessPostExecution, to manage credit deductions and state updates for gasless transactions after execution, improving clarity and separation of concerns. - Updated stateTransition struct to include gasStationStorageSlots for better management of gasless transaction data.
- Introduced a new test file to verify that gasless transactions consume the same amount of gas as standard transactions. - Implemented test cases that create a simulated environment for both transaction types, ensuring accurate gas usage comparison and credit deduction verification. - Enhanced the clarity and reliability of gasless transaction handling through comprehensive testing.
- Modified existing test cases in rollup_cost_test.go to utilize a new transaction variant, emptyTxWithGasPrice, ensuring accurate gas and fee calculations. - Enhanced the test coverage for cost functions by incorporating gas price scenarios, improving the reliability of the cost estimation logic. - Adjusted the ordering_test.go to ensure gas fee caps are always non-zero, enhancing the robustness of transaction price sorting tests.
- Enabled the test job in the build_and_publish.yml workflow to run Go tests. - Updated the Go version to 1.22.0 for improved compatibility and performance. - Ensured the release job depends on the test job for better workflow management.
- Updated the transaction creation in testEIP155Transition to use a fixed gas price of 1 instead of nil, ensuring accurate simulation of non gasless transaction costs. - This change enhances the reliability of the tests by providing a consistent gas price scenario for better validation of EIP155 transitions.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
core/state_transition.go (1)
656-660: Credit deduction after refunds — addresses earlier overchargeMoved post returnGas(); now charges on final UsedGas. Good fix.
🧹 Nitpick comments (2)
core/state_transition.go (2)
392-397: Add tracer signal for “free gas” grant in gasless preCheckMirror buyGas()’s GasChangeTxInitialBalance so traces stay consistent.
Apply:
if st.msg.IsGaslessTx { st.initialGas = st.msg.GasLimit st.gasRemaining += st.msg.GasLimit + if st.evm.Config.Tracer != nil && st.evm.Config.Tracer.OnGasChange != nil { + st.evm.Config.Tracer.OnGasChange(0, st.msg.GasLimit, tracing.GasChangeTxInitialBalance) + } return st.gp.SubGas(st.msg.GasLimit) }
846-894: Gasless post-exec accounting is sound; two tiny polish itemsLogic/order is correct (read → subtract final UsedGas → clamp → write; then single-use mark). Two small readability tweaks:
- Prefer non-zero check for feature flag bytes.
- Use Keccak256Hash on two slices to avoid an intermediate append.
Apply:
- isSingleUseEnabled := singleUseEnabled[31] == 0x01 + isSingleUseEnabled := singleUseEnabled[31] != 0x00 - userCombined := append(userKeyPadded, mapBaseSlotPadded...) - userUsedSlotHash := crypto.Keccak256Hash(userCombined) + userUsedSlotHash := crypto.Keccak256Hash(userKeyPadded, mapBaseSlotPadded)If helpful, I can add a focused test asserting:
- credits deducted equals final UsedGas (post-refund),
- single-use slot set for msg.From,
- coinbase balance unchanged for gasless.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
core/state_transition.go(8 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Sledro
PR: lightlink-network/ll-geth#1
File: params/protocol_params.go:0-0
Timestamp: 2025-06-04T23:33:50.744Z
Learning: For the gasless transaction system (LLIP-2), the GasStation contract address is temporarily configured via environment variable for devnet-2 testing, but will be predeployed for devnet-3 onwards to ensure deterministic addresses across environments.
📚 Learning: 2025-06-06T12:55:40.489Z
Learnt from: Sledro
PR: lightlink-network/ll-geth#1
File: core/state_transition.go:379-391
Timestamp: 2025-06-06T12:55:40.489Z
Learning: In gasless transactions, the isGaslessTx() method implementations across transaction types (DynamicFeeTx, LegacyTx) already enforce that gas prices and fee caps must be zero. This means returnGas() will not mint ETH for gasless transactions since gasRemaining * 0 = 0, but the gas pool logic (st.gp.AddGas) still executes to properly return gas to the block gas counter.
Applied to files:
core/state_transition.go
🧬 Code graph analysis (1)
core/state_transition.go (4)
core/gas_station.go (2)
GasStationStorageSlots(34-41)ValidateGaslessTx(93-174)core/vm/interface.go (1)
StateDB(31-104)params/protocol_params.go (1)
GasStationAddress(237-237)crypto/crypto.go (1)
Keccak256Hash(97-104)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Go Tests
🔇 Additional comments (6)
core/state_transition.go (6)
29-29: Import for Keccak slot hashing looks goodUsed by handleGaslessPostExecution() for nested mapping slot calc. No concerns.
173-174: Message gains IsGaslessTx flag — correct placeField placement and semantics are consistent with usage across the file.
198-199: Propagate tx.IsGaslessTx in TransactionToMessageCorrect propagation; avoids recomputing per-call.
222-235: Early gasless pre-validation and slot caching is cleanValidates before metering and carries slots into stateTransition; good separation.
260-267: stateTransition: new GasStation slots fieldAppropriate ownership; keeps slot recomputation out of hot paths.
664-672: Skip coinbase payment for gasless txEarly return avoids unintended tipping and OP fee splits for gasless. Looks right.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.github/workflows/build_and_publish.yml (1)
59-66: Simplify Buildx caching with GHA cache; drop manual /tmp cache dance.Reduces steps and avoids fragile cache moves.
Apply:
- - name: Docker cache layers - uses: actions/cache@v4 - with: - path: /tmp/.buildx-cache - key: ${{ runner.os }}-single-buildx-${{ github.sha }} - restore-keys: | - ${{ runner.os }}-single-buildx- name: Push Image uses: docker/build-push-action@v5 with: context: . push: true tags: ${{ steps.login-ecr.outputs.registry }}/${{ env.ECR_REPOSITORY }}:${{ steps.get-tag.outputs.version }} - cache-from: type=local,src=/tmp/.buildx-cache - cache-to: type=local,dest=/tmp/.buildx-cache-new + cache-from: type=gha + cache-to: type=gha,mode=max build-args: VERSION=${{ steps.get-tag.outputs.version }}- - name: Move cache - run: | - rm -rf /tmp/.buildx-cache - mv /tmp/.buildx-cache-new /tmp/.buildx-cacheAlso applies to: 76-85, 86-89
core/state_transition.go (2)
392-397: Optional: add tracer hook for initial-gas on gasless.Parity with buyGas path improves observability.
Apply:
if st.msg.IsGaslessTx { st.initialGas = st.msg.GasLimit st.gasRemaining += st.msg.GasLimit + if st.evm.Config.Tracer != nil && st.evm.Config.Tracer.OnGasChange != nil { + st.evm.Config.Tracer.OnGasChange(0, st.msg.GasLimit, tracing.GasChangeTxInitialBalance) + } return st.gp.SubGas(st.msg.GasLimit) }
867-874: Make single-use flag check robust against encoding variations.Avoid relying on a specific byte index; treat any non-zero as enabled.
Apply:
- singleUseEnabled := st.state.GetState(params.GasStationAddress, slots.SingleUseEnabledSlotHash) - isSingleUseEnabled := singleUseEnabled[31] == 0x01 + singleUseEnabled := st.state.GetState(params.GasStationAddress, slots.SingleUseEnabledSlotHash) + isSingleUseEnabled := new(big.Int).SetBytes(singleUseEnabled[:]).Sign() != 0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/build_and_publish.yml(1 hunks)core/state_transition.go(8 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Sledro
PR: lightlink-network/ll-geth#1
File: params/protocol_params.go:0-0
Timestamp: 2025-06-04T23:33:50.744Z
Learning: For the gasless transaction system (LLIP-2), the GasStation contract address is temporarily configured via environment variable for devnet-2 testing, but will be predeployed for devnet-3 onwards to ensure deterministic addresses across environments.
📚 Learning: 2025-06-06T12:55:40.489Z
Learnt from: Sledro
PR: lightlink-network/ll-geth#1
File: core/state_transition.go:379-391
Timestamp: 2025-06-06T12:55:40.489Z
Learning: In gasless transactions, the isGaslessTx() method implementations across transaction types (DynamicFeeTx, LegacyTx) already enforce that gas prices and fee caps must be zero. This means returnGas() will not mint ETH for gasless transactions since gasRemaining * 0 = 0, but the gas pool logic (st.gp.AddGas) still executes to properly return gas to the block gas counter.
Applied to files:
core/state_transition.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build Image & Push to ECR
🔇 Additional comments (12)
.github/workflows/build_and_publish.yml (2)
67-75: Version tagging step looks good.
51-58: ECR login step is correct.core/state_transition.go (10)
29-29: Import for Keccak usage is appropriate.
173-174: Message gains IsGaslessTx flag — OK.
198-199: TransactionToMessage propagates IsGaslessTx — OK.
222-231: Pre-validate gasless before metering — good placement.Using evm.StateDB avoids brittle casts; aligns with prior learnings.
260-267: State transition holds GasStation slots — OK.
656-660: Post-refund credit deduction is correct.Deducting after calcRefund()/returnGas avoids over-charging. Matches the fix discussed earlier.
664-672: Skip coinbase payout for gasless — correct.As gas price/fee caps are zero for gasless, skipping miner tips is consistent with design.
851-865: Credit math with underflow clamp — good.Re-reading and clamping to zero prevents state corruption on edge cases.
881-894: Event emission intentionally commented — OK for now.Enable later when topics/ABI are finalized.
870-879: Mapping slot basis confirmedThe
UsedAddressesMapBaseSlotHashvalue is derived usingcommon.BigToHashon the computed slot index (structBaseSlot + 5), producing the raw 32-byte big-endian slot number rather than a hashed value. Consequently, the subsequentcrypto.Keccak256Hash(userKeyPadded || mapBaseSlotPadded)correctly computeskeccak256(key ‖ slot)per the Solidity mapping specification, and there is no double-hashing.
| env: | ||
| RELEASE_REVISION: ${{ github.sha }} | ||
| AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} | ||
| AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} | ||
| AWS_REGION: ${{ secrets.AWS_REGION }} | ||
| ECR_REPOSITORY: ll-geth | ||
|
|
There was a problem hiding this comment.
Stop exporting AWS secrets as top-level env; pass secrets directly to the action.
Top-level env propagates to all steps (including shell), increasing leak risk. Feed secrets directly into aws-actions instead.
Apply these diffs:
-env:
- RELEASE_REVISION: ${{ github.sha }}
- AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
- AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
- AWS_REGION: ${{ secrets.AWS_REGION }}
- ECR_REPOSITORY: ll-geth
+env:
+ ECR_REPOSITORY: ll-geth - name: Configure AWS credentials
uses: aws-actions/configure-aws-credentials@v4
with:
- aws-access-key-id: ${{ env.AWS_ACCESS_KEY_ID }}
- aws-secret-access-key: ${{ env.AWS_SECRET_ACCESS_KEY }}
- aws-region: ${{ env.AWS_REGION }}
+ aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }}
+ aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
+ aws-region: ${{ secrets.AWS_REGION }}Also applies to: 44-50
🤖 Prompt for AI Agents
In .github/workflows/build_and_publish.yml around lines 12-18 (and similarly
lines 44-50), the workflow exports AWS secrets as top-level env which exposes
them to every step; remove AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, and
AWS_REGION (and any secret ECR_* values) from the top-level env block and
instead pass them directly to the aws-actions step(s) via the step's with: or
env: entries (e.g., aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }},
aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }}, aws-region: ${{
secrets.AWS_REGION }}), keeping only non-secret values like RELEASE_REVISION at
the top level; update both occurrences (lines 12-18 and 44-50) accordingly.
| release: | ||
| # needs: test | ||
| name: Build Image & Push to ECR | ||
| runs-on: ubuntu-latest | ||
| steps: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Gate ECR push on push-events to main/tags only.
Avoid attempting to push on pull_request events and from forks.
Apply:
release:
+ if: github.event_name == 'push' && (startsWith(github.ref, 'refs/heads/main') || startsWith(github.ref, 'refs/tags/'))
# needs: test
name: Build Image & Push to ECR
runs-on: ubuntu-latest
+ permissions:
+ contents: read📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| release: | |
| # needs: test | |
| name: Build Image & Push to ECR | |
| runs-on: ubuntu-latest | |
| steps: | |
| release: | |
| if: github.event_name == 'push' && (startsWith(github.ref, 'refs/heads/main') || startsWith(github.ref, 'refs/tags/')) | |
| # needs: test | |
| name: Build Image & Push to ECR | |
| runs-on: ubuntu-latest | |
| permissions: | |
| contents: read | |
| steps: |
🤖 Prompt for AI Agents
In .github/workflows/build_and_publish.yml around lines 34-38, the release job
currently runs for events that include pull_request/forks; change its gating so
ECR push runs only for push events to main or tag refs. Update the workflow
triggers or add a job-level condition: require github.event_name == 'push' and
(github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/')), so
the job is skipped for pull_request events and forked PRs; ensure any
push-to-ECR steps are behind that condition.
lightlinkbranch is based onv1.101503.2tag which is deployed ondevnet-2Summary by CodeRabbit
New Features
Chores
Tests