Skip to content

Commit

Permalink
Merge pull request #50 from FastLane-Labs/if-else-comments
Browse files Browse the repository at this point in the history
Refactor comments in if-else blocks
  • Loading branch information
jj1980a authored Nov 18, 2023
2 parents d930898 + 6e2d297 commit 994683b
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 57 deletions.
19 changes: 12 additions & 7 deletions src/contracts/atlas/Atlas.sol
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,9 @@ contract Atlas is Escrow {
}

// bundler checks
// user is bundling their own operation - check allowed and valid dapp signature/callchainhash
if (msg.sender == userOp.from) {
// user is bundling their own operation - check allowed and valid dapp signature/callchainhash

if (!dConfig.callConfig.allowsUserBundler()) {
return ValidCallsResult.UnknownBundlerNotAllowed;
}
Expand All @@ -277,8 +278,9 @@ contract Atlas is Escrow {
{
return ValidCallsResult.InvalidSequence;
}
} // dapp is bundling - always allowed, check valid user/dapp signature and callchainhash
else if (msg.sender == dAppOp.from) {
} else if (msg.sender == dAppOp.from) {
// dapp is bundling - always allowed, check valid user/dapp signature and callchainhash

// check dapp signature
if (!IAtlasVerification(VERIFICATION).verifyDApp(dConfig, dAppOp)) {
bool bypass = isSimulation && dAppOp.signature.length == 0;
Expand All @@ -300,8 +302,10 @@ contract Atlas is Escrow {
{
return ValidCallsResult.InvalidSequence;
}
} // potentially the winning solver is bundling - check that its allowed and only need to verify user signature
else if (msg.sender == solverOps[0].from && solverOps.length == 1) {
} else if (msg.sender == solverOps[0].from && solverOps.length == 1) {
// potentially the winning solver is bundling - check that its allowed and only need to verify user
// signature

// check if protocol allows it
if (!dConfig.callConfig.allowsSolverBundler()) {
return ValidCallsResult.DAppSignatureInvalid;
Expand All @@ -324,8 +328,9 @@ contract Atlas is Escrow {
return ValidCallsResult.InvalidSequence;
}
}
} // check if protocol allows unknown bundlers, and verify all signatures if they do
else if (dConfig.callConfig.allowsUnknownBundler()) {
} else if (dConfig.callConfig.allowsUnknownBundler()) {
// check if protocol allows unknown bundlers, and verify all signatures if they do

// check dapp signature
if (!IAtlasVerification(VERIFICATION).verifyDApp(dConfig, dAppOp)) {
bool bypass = isSimulation && dAppOp.signature.length == 0;
Expand Down
3 changes: 1 addition & 2 deletions src/contracts/atlas/Escrow.sol
Original file line number Diff line number Diff line change
Expand Up @@ -224,9 +224,8 @@ abstract contract Escrow is AtlETH {

// Save the escrow data back into storage
_escrowAccountData[solverOp.from] = solverEscrow;

// Check if need to save escrowData due to nonce update but not gasRebate
} else if (result & EscrowBits._NO_NONCE_UPDATE == 0) {
// Need to save escrowData due to nonce update but not gasRebate
_escrowAccountData[solverOp.from].nonce = solverEscrow.nonce;
}
}
Expand Down
26 changes: 16 additions & 10 deletions src/contracts/atlas/GasAccountingLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,14 @@ contract GasAccountingLib is Storage, FastLaneErrorsEvents {
if (partyLedger.status == LedgerStatus.Inactive) partyLedger.status = LedgerStatus.Active;

if (partyLedger.requested < 0) {
// CASE: still in deficit
if (partyLedger.requested + amount < 0) {
// CASE: still in deficit

partyLedger.requested += amount;
amount = 0;

// CASE: surplus
} else {
// CASE: surplus

amount += partyLedger.requested;
partyLedger.requested = 0;
}
Expand Down Expand Up @@ -391,6 +392,7 @@ contract GasAccountingLib is Storage, FastLaneErrorsEvents {

if (solverSuccessful) {
// CASE: Delete the balances on the older ledger

// TODO: Pretty sure we can skip this since it gets ignored and deleted later
partyLedger.balance = 0;
partyLedger.contributed = 0;
Expand All @@ -399,6 +401,7 @@ contract GasAccountingLib is Storage, FastLaneErrorsEvents {
ledgers[builderIndex] = partyLedger; // Proxy status stays
} else {
// CASE: Undo the balance adjustments for the next solver

sLedger.balance -= partyLedger.balance;
sLedger.contributed -= partyLedger.contributed;
sLedger.requested -= partyLedger.requested;
Expand All @@ -422,6 +425,7 @@ contract GasAccountingLib is Storage, FastLaneErrorsEvents {

if (solverSuccessful) {
// CASE: Delete the balances on the older ledger

// TODO: Pretty sure we can skip this since it gets ignored and deleted later
partyLedger.balance = 0;
partyLedger.contributed = 0;
Expand All @@ -430,6 +434,7 @@ contract GasAccountingLib is Storage, FastLaneErrorsEvents {
ledgers[bundlerIndex] = partyLedger; // Proxy status stays
} else {
// CASE: Undo the balance adjustments for the next solver

sLedger.balance -= partyLedger.balance;
sLedger.contributed -= partyLedger.contributed;
sLedger.requested -= partyLedger.requested;
Expand Down Expand Up @@ -585,14 +590,13 @@ contract GasAccountingLib is Storage, FastLaneErrorsEvents {

if (partyLedger.contributed < 0) revert NoUnfilledRequests();

// Only tally totals from non-proxies
if (uint256(partyLedger.proxy) == i) {
// Only tally totals from non-proxies
totalBalanceDelta += partyLedger.balance;
totalRequests += partyLedger.requested;
totalContributions += partyLedger.contributed;

// Mark inactive if proxy
} else {
// Mark inactive if proxy
activeParties = activeParties.markInactive(Party(i));
}

Expand Down Expand Up @@ -687,18 +691,20 @@ contract GasAccountingLib is Storage, FastLaneErrorsEvents {

Ledger memory partyLedger = parties[i];

// CASE: Some Requests still in Deficit
if (partyLedger.requested < 0 && partyLedger.contributed > 0) {
// CASE: Contributions > Requests
// CASE: Some Requests still in Deficit

if (partyLedger.contributed + partyLedger.requested > 0) {
// CASE: Contributions > Requests

totalRequests -= partyLedger.requested; // subtracting a negative
totalContributions += partyLedger.requested; // adding a negative

partyLedger.contributed += partyLedger.requested; // adding a negative
partyLedger.requested = 0;

// CASE: Requests >= Contributions
} else {
// CASE: Requests >= Contributions

totalRequests += partyLedger.contributed; // adding a positive
totalContributions -= partyLedger.contributed; // subtracting a positive

Expand Down
13 changes: 5 additions & 8 deletions src/contracts/common/ExecutionBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -191,28 +191,25 @@ contract Base {
uint256 pIndex = uint256(party);
// MEDIAN INDEX
if (pIndex < uint256(Party.Solver)) {
// CASE: BUILDER
if (party == Party.Builder) {
// CASE: BUILDER
return block.coinbase;

// CASE: BUNDLER
} else {
// CASE: BUNDLER
return tx.origin; // TODO: This may be unreliable for smart wallet integrations.
}
} else if (pIndex > uint256(Party.Solver)) {
// CASE: USER
if (party == Party.User) {
// CASE: USER
return _user();

// CASE: DAPP
} else {
// CASE: DAPP
return _control();
}

} else {
// CASE: SOLVER
// NOTE: Currently unimplemented
// TODO: check if this is a SolverOp phase and use assembly to grab the solverOp.from from calldata
} else {
revert("ERR-EB090 SolverPartyUnimplemented");
}
}
Expand Down
11 changes: 3 additions & 8 deletions src/contracts/examples/v4-example/UniV4Hook.sol
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,14 @@ contract UniV4Hook is V4DAppControl {

EscrowKey memory escrowKey = ISafetyLocks(escrow).getLockState();

// Case:
// User call
if (escrowKey.lockState == SafetyBits._LOCKED_X_USER_X_UNSET) {
// Case: User call
// Sender = ExecutionEnvironment

// Verify that the pool is valid for the user to trade in.
require(keccak256(abi.encode(key, sender)) == hashLock, "ERR-H02 InvalidSwapper");

// Case:
// Solver call
} else if (escrowKey.lockState == SafetyBits._LOCKED_X_SOLVERS_X_VERIFIED) {
// Case: Solver call
// Sender = Solver contract
// NOTE: This lockState verifies that the user's transaction has already
// been executed.
Expand All @@ -98,10 +95,8 @@ contract UniV4Hook is V4DAppControl {

// Verify that the pool is valid for a solver to trade in.
require(hashLock == keccak256(abi.encode(key, escrowKey.approvedCaller)), "ERR-H04 InvalidPoolKey");

// Case:
// Other call
} else {
// Case: Other call
// Determine if the sequenced order was processed earlier in the block
bytes32 sequenceKey = keccak256(
abi.encodePacked(
Expand Down
28 changes: 10 additions & 18 deletions src/contracts/libraries/GasParties.sol
Original file line number Diff line number Diff line change
Expand Up @@ -105,25 +105,21 @@ library PartyMath {

// median index party = Solver
if (pIndex > uint256(Party.Solver)) {
// CASE: DAPP
if (pIndex == uint256(Party.DApp)) {
// CASE: DAPP
return lock & VALID_PHASES_DAPP_CONTRIBUTION != 0;

// CASE: USER
} else {
// CASE: USER
return lock & VALID_PHASES_USER_CONTRIBUTION != 0;
}

// CASE: SOLVER
} else if (pIndex == uint256(Party.Solver)) {
// CASE: SOLVER
return lock & VALID_PHASES_SOLVER_CONTRIBUTION != 0;

// CASE: BUNDLER
} else if (pIndex == uint256(Party.Bundler)) {
// CASE: BUNDLER
return lock & VALID_PHASES_BUNDLER_CONTRIBUTION != 0;

// CASE: BUILDER
} else {
// CASE: BUILDER
return lock & VALID_PHASES_BUILDER_CONTRIBUTION != 0;
}
}
Expand All @@ -134,25 +130,21 @@ library PartyMath {

// median index party = Solver
if (pIndex > uint256(Party.Solver)) {
// CASE: DAPP
if (pIndex == uint256(Party.DApp)) {
// CASE: DAPP
return lock & VALID_PHASES_DAPP_REQUEST != 0;

// CASE: USER
} else {
// CASE: USER
return lock & VALID_PHASES_USER_REQUEST != 0;
}

// CASE: SOLVER
} else if (pIndex == uint256(Party.Solver)) {
// CASE: SOLVER
return lock & VALID_PHASES_SOLVER_REQUEST != 0;

// CASE: BUNDLER
} else if (pIndex == uint256(Party.Bundler)) {
// CASE: BUNDLER
return lock & VALID_PHASES_BUNDLER_REQUEST != 0;

// CASE: BUILDER
} else {
// CASE: BUILDER
return lock & VALID_PHASES_BUILDER_REQUEST != 0;
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/contracts/solver/SolverBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -65,19 +65,19 @@ contract SolverBase is Test {
_;

// Handle bid payment

// Ether balance
if (bidToken == address(0)) {
// Ether balance

uint256 ethOwed = bidAmount + msg.value;

if (ethOwed > address(this).balance) {
IWETH9(WETH_ADDRESS).withdraw(ethOwed - address(this).balance);
}

SafeTransferLib.safeTransferETH(msg.sender, bidAmount);

// ERC20 balance
} else {
// ERC20 balance

if (msg.value > address(this).balance) {
IWETH9(WETH_ADDRESS).withdraw(msg.value - address(this).balance);
}
Expand Down

0 comments on commit 994683b

Please sign in to comment.