Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Revert "Revert "Merge pull request #2224 from joshuef/RangeBasedGets""" #2369

Merged
merged 4 commits into from
Nov 2, 2024
Merged

Revert "Revert "Revert "Merge pull request #2224 from joshuef/RangeBasedGets""" #2369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
05b7710
Revert "Revert "Revert "Merge pull request #2224 from joshuef/RangeBa…
maqi Oct 31, 2024
4af2ed5
fix(node): ignore timestamp elapsed call failure
maqi Oct 29, 2024
ef9b81c
fix(node): fairly pick verification candidates
maqi Oct 29, 2024
5146831
chore(node): create a RecordCache struct
maqi Oct 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
245 changes: 115 additions & 130 deletions .github/workflows/merge.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,19 +535,15 @@ jobs:
# platform: ${{ matrix.os }}
# build: true

# # incase the faucet is not ready yet
# - name: 30s sleep for faucet completion
# run: sleep 30

# - name: Check SAFE_PEERS was set
# shell: bash
# run: |
# if [[ -z "$SAFE_PEERS" ]]; then
# echo "The SAFE_PEERS variable has not been set"
# exit 1
# else
# echo "SAFE_PEERS has been set to $SAFE_PEERS"
# fi
# - name: Check SAFE_PEERS was set
# shell: bash
# run: |
# if [[ -z "$SAFE_PEERS" ]]; then
# echo "The SAFE_PEERS variable has not been set"
# exit 1
# else
# echo "SAFE_PEERS has been set to $SAFE_PEERS"
# fi

# - name: execute token_distribution tests
# run: cargo test --release --features=local,distribution token_distribution -- --nocapture --test-threads=1
Expand Down Expand Up @@ -798,7 +794,7 @@ jobs:
uses: maidsafe/sn-local-testnet-action@main
with:
action: stop
log_file_prefix: safe_test_logs_data_location_routing_table
log_file_prefix: safe_test_logs_data_location
platform: ${{ matrix.os }}

- name: Verify restart of nodes using rg
Expand Down Expand Up @@ -899,15 +895,15 @@ jobs:
# echo "SAFE_PEERS has been set to $SAFE_PEERS"
# fi

# - name: Create and fund a wallet first time
# run: |
# ~/safe --log-output-dest=data-dir wallet create --no-password
# ~/faucet --log-output-dest=data-dir send 100000000 $(~/safe --log-output-dest=data-dir wallet address | tail -n 1) | tail -n 1 1>first.txt
# echo "----------"
# cat first.txt
# env:
# SN_LOG: "all"
# timeout-minutes: 5
# - name: Create and fund a wallet first time
# run: |
# ~/safe --log-output-dest=data-dir wallet create --no-password
# ~/faucet --log-output-dest=data-dir send 100000000 $(~/safe --log-output-dest=data-dir wallet address | tail -n 1) | tail -n 1 1>first.txt
# echo "----------"
# cat first.txt
# env:
# SN_LOG: "all"
# timeout-minutes: 5

# - name: Move faucet log to the working folder
# run: |
Expand Down Expand Up @@ -1158,6 +1154,7 @@ jobs:
# runs-on: ubuntu-latest
# env:
# CLIENT_DATA_PATH: /home/runner/.local/share/safe/autonomi

# steps:
# - uses: actions/checkout@v4

Expand Down Expand Up @@ -1234,61 +1231,44 @@ jobs:
# echo "SAFE_PEERS has been set to $SAFE_PEERS"
# fi

# - name: Sleep 15s
# shell: bash
# run: sleep 15

# - name: Check faucet has been funded
# shell: bash
# run: |
# cash_note_count=$(ls -l /home/runner/.local/share/safe/test_faucet/wallet/cash_notes/ | wc -l)
# echo $cash_note_count
# if [ "$cash_note_count" -eq 0 ]; then
# echo "Error: Expected at least 1 cash note, but found $cash_note_count"
# exit 1
# fi

# - name: Create and fund a wallet to pay for files storage
# run: |
# ./target/release/safe --log-output-dest=data-dir wallet create --no-password
# ./target/release/faucet --log-output-dest=data-dir send 100000000 $(./target/release/safe --log-output-dest=data-dir wallet address | tail -n 1) | tail -n 1 > transfer_hex
# ./target/release/safe --log-output-dest=data-dir wallet receive --file transfer_hex
# env:
# SN_LOG: "all"
# timeout-minutes: 5
# - name: Create and fund a wallet to pay for files storage
# run: |
# ./target/release/safe --log-output-dest=data-dir wallet create --no-password
# ./target/release/faucet --log-output-dest=data-dir send 100000000 $(./target/release/safe --log-output-dest=data-dir wallet address | tail -n 1) | tail -n 1 > transfer_hex
# ./target/release/safe --log-output-dest=data-dir wallet receive --file transfer_hex
# env:
# SN_LOG: "all"
# timeout-minutes: 5

# - name: Start a client to upload first file
# run: ./target/release/safe --log-output-dest=data-dir files upload "./test_data_1.tar.gz" --retry-strategy quick
# env:
# SN_LOG: "all"
# timeout-minutes: 5

# - name: Check current directories
# run: |
# pwd
# ls $CLIENT_DATA_PATH/ -l
# ls $CLIENT_DATA_PATH/wallet -l
# ls $CLIENT_DATA_PATH/wallet/cash_notes -l
# timeout-minutes: 1

# - name: Ensure no leftover cash_notes and payment files
# run: |
# expected_cash_notes_files="1"
# expected_payment_files="0"
# cash_note_files=$(ls $CLIENT_DATA_PATH/wallet/cash_notes | wc -l)
# echo "Find $cash_note_files cash_note files"
# if [ $expected_cash_notes_files -lt $cash_note_files ]; then
# echo "Got too many cash_note files leftover: $cash_note_files"
# exit 1
# fi
# ls $CLIENT_DATA_PATH/wallet/payments -l
# payment_files=$(ls $CLIENT_DATA_PATH/wallet/payments | wc -l)
# if [ $expected_payment_files -lt $payment_files ]; then
# echo "Got too many payment files leftover: $payment_files"
# exit 1
# fi

# timeout-minutes: 10
# - name: Ensure no leftover cash_notes and payment files
# run: |
# expected_cash_notes_files="1"
# expected_payment_files="0"
# pwd
# ls $CLIENT_DATA_PATH/ -l
# ls $CLIENT_DATA_PATH/wallet -l
# ls $CLIENT_DATA_PATH/wallet/cash_notes -l
# cash_note_files=$(ls $CLIENT_DATA_PATH/wallet/cash_notes | wc -l)
# echo "Find $cash_note_files cash_note files"
# if [ $expected_cash_notes_files -lt $cash_note_files ]; then
# echo "Got too many cash_note files leftover: $cash_note_files"
# exit 1
# fi
# ls $CLIENT_DATA_PATH/wallet/payments -l
# payment_files=$(ls $CLIENT_DATA_PATH/wallet/payments | wc -l)
# if [ $expected_payment_files -lt $payment_files ]; then
# echo "Got too many payment files leftover: $payment_files"
# exit 1
# fi
# env:
# CLIENT_DATA_PATH: /home/runner/.local/share/safe/client
# timeout-minutes: 10

# - name: Wait for certain period
# run: sleep 300
Expand All @@ -1300,77 +1280,82 @@ jobs:
# SN_LOG: "all"
# timeout-minutes: 10

# - name: Ensure no leftover cash_notes and payment files
# run: |
# expected_cash_notes_files="1"
# expected_payment_files="0"
# pwd
# ls $CLIENT_DATA_PATH/ -l
# ls $CLIENT_DATA_PATH/wallet -l
# ls $CLIENT_DATA_PATH/wallet/cash_notes -l
# cash_note_files=$(find $CLIENT_DATA_PATH/wallet/cash_notes -type f | wc -l)
# if (( $(echo "$cash_note_files > $expected_cash_notes_files" | bc -l) )); then
# echo "Got too many cash_note files leftover: $cash_note_files when we expected $expected_cash_notes_files"
# exit 1
# fi
# ls $CLIENT_DATA_PATH/wallet/payments -l
# payment_files=$(find $CLIENT_DATA_PATH/wallet/payments -type f | wc -l)
# if (( $(echo "$payment_files > $expected_payment_files" | bc -l) )); then
# echo "Got too many payment files leftover: $payment_files"
# exit 1
# fi
# timeout-minutes: 10
# - name: Ensure no leftover cash_notes and payment files
# run: |
# expected_cash_notes_files="1"
# expected_payment_files="0"
# pwd
# ls $CLIENT_DATA_PATH/ -l
# ls $CLIENT_DATA_PATH/wallet -l
# ls $CLIENT_DATA_PATH/wallet/cash_notes -l
# cash_note_files=$(find $CLIENT_DATA_PATH/wallet/cash_notes -type f | wc -l)
# if (( $(echo "$cash_note_files > $expected_cash_notes_files" | bc -l) )); then
# echo "Got too many cash_note files leftover: $cash_note_files when we expected $expected_cash_notes_files"
# exit 1
# fi
# ls $CLIENT_DATA_PATH/wallet/payments -l
# payment_files=$(find $CLIENT_DATA_PATH/wallet/payments -type f | wc -l)
# if (( $(echo "$payment_files > $expected_payment_files" | bc -l) )); then
# echo "Got too many payment files leftover: $payment_files"
# exit 1
# fi
# env:
# CLIENT_DATA_PATH: /home/runner/.local/share/safe/client
# timeout-minutes: 10

# - name: Wait for certain period
# run: sleep 300
# timeout-minutes: 6

# # Start a different client to avoid local wallet slow down with more payments handled.
# - name: Start a different client
# run: |
# pwd
# mv $CLIENT_DATA_PATH $SAFE_DATA_PATH/client_first
# ls -l $SAFE_DATA_PATH
# ls -l $SAFE_DATA_PATH/client_first
# mkdir $SAFE_DATA_PATH/client
# ls -l $SAFE_DATA_PATH
# mv $SAFE_DATA_PATH/client_first/logs $CLIENT_DATA_PATH/logs
# ls -l $CLIENT_DATA_PATH
# ./target/release/safe --log-output-dest=data-dir wallet create --no-password
# ./target/release/faucet --log-output-dest=data-dir send 100000000 $(./target/release/safe --log-output-dest=data-dir wallet address | tail -n 1) | tail -n 1 > transfer_hex
# ./target/release/safe --log-output-dest=data-dir wallet receive --file transfer_hex
# env:
# SN_LOG: "all"
# SAFE_DATA_PATH: /home/runner/.local/share/safe
# timeout-minutes: 25
# # Start a different client to avoid local wallet slow down with more payments handled.
# - name: Start a different client
# run: |
# pwd
# mv $CLIENT_DATA_PATH $SAFE_DATA_PATH/client_first
# ls -l $SAFE_DATA_PATH
# ls -l $SAFE_DATA_PATH/client_first
# mkdir $SAFE_DATA_PATH/client
# ls -l $SAFE_DATA_PATH
# mv $SAFE_DATA_PATH/client_first/logs $CLIENT_DATA_PATH/logs
# ls -l $CLIENT_DATA_PATH
# ./target/release/safe --log-output-dest=data-dir wallet create --no-password
# ./target/release/faucet --log-output-dest=data-dir send 100000000 $(./target/release/safe --log-output-dest=data-dir wallet address | tail -n 1) | tail -n 1 > transfer_hex
# ./target/release/safe --log-output-dest=data-dir wallet receive --file transfer_hex
# env:
# SN_LOG: "all"
# SAFE_DATA_PATH: /home/runner/.local/share/safe
# CLIENT_DATA_PATH: /home/runner/.local/share/safe/client
# timeout-minutes: 25

# - name: Use second client to upload third file
# run: ./target/release/safe --log-output-dest=data-dir files upload "./test_data_3.tar.gz" --retry-strategy quick
# env:
# SN_LOG: "all"
# timeout-minutes: 10

# - name: Ensure no leftover cash_notes and payment files
# run: |
# expected_cash_notes_files="1"
# expected_payment_files="0"
# pwd
# ls $CLIENT_DATA_PATH/ -l
# ls $CLIENT_DATA_PATH/wallet -l
# ls $CLIENT_DATA_PATH/wallet/cash_notes -l
# cash_note_files=$(ls $CLIENT_DATA_PATH/wallet/cash_notes | wc -l)
# echo "Find $cash_note_files cash_note files"
# if [ $expected_cash_notes_files -lt $cash_note_files ]; then
# echo "Got too many cash_note files leftover: $cash_note_files"
# exit 1
# fi
# ls $CLIENT_DATA_PATH/wallet/payments -l
# payment_files=$(ls $CLIENT_DATA_PATH/wallet/payments | wc -l)
# if [ $expected_payment_files -lt $payment_files ]; then
# echo "Got too many payment files leftover: $payment_files"
# exit 1
# fi
# timeout-minutes: 10
# - name: Ensure no leftover cash_notes and payment files
# run: |
# expected_cash_notes_files="1"
# expected_payment_files="0"
# pwd
# ls $CLIENT_DATA_PATH/ -l
# ls $CLIENT_DATA_PATH/wallet -l
# ls $CLIENT_DATA_PATH/wallet/cash_notes -l
# cash_note_files=$(ls $CLIENT_DATA_PATH/wallet/cash_notes | wc -l)
# echo "Find $cash_note_files cash_note files"
# if [ $expected_cash_notes_files -lt $cash_note_files ]; then
# echo "Got too many cash_note files leftover: $cash_note_files"
# exit 1
# fi
# ls $CLIENT_DATA_PATH/wallet/payments -l
# payment_files=$(ls $CLIENT_DATA_PATH/wallet/payments | wc -l)
# if [ $expected_payment_files -lt $payment_files ]; then
# echo "Got too many payment files leftover: $payment_files"
# exit 1
# fi
# env:
# CLIENT_DATA_PATH: /home/runner/.local/share/safe/client
# timeout-minutes: 10

# - name: Stop the local network and upload logs
# if: always()
Expand Down
28 changes: 16 additions & 12 deletions sn_evm/src/data_payments.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,17 +244,29 @@ impl PaymentQuote {
return false;
}

// TODO: Double check if this applies, as this will prevent a node restart with same ID
if new_quote.quoting_metrics.received_payment_count
< old_quote.quoting_metrics.received_payment_count
{
info!("claimed received_payment_count out of sequence");
return false;
}

let old_elapsed = if let Ok(elapsed) = old_quote.timestamp.elapsed() {
elapsed
} else {
info!("timestamp failure");
return false;
// The elapsed call could fail due to system clock change
// hence consider the verification succeeded.
info!("old_quote timestamp elapsed call failure");
return true;
};
let new_elapsed = if let Ok(elapsed) = new_quote.timestamp.elapsed() {
elapsed
} else {
info!("timestamp failure");
return false;
// The elapsed call could fail due to system clock change
// hence consider the verification succeeded.
info!("new_quote timestamp elapsed call failure");
return true;
Comment on lines +247 to +269
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not 100% sure about this, @mickvandijke could you confirm this makes sense?

Copy link
Contributor

@mickvandijke mickvandijke Nov 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand why these checks are done at all. Maybe I'm missing something. But wouldn't we only need to verify a quote's signature to check if it is made by this peer? Why do we need to store historical quotes and compare with them?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was to confirm a quote given by other node are not faked.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The quote signature being valid already rules out that the quote could have been faked by another node or client. So I think we can get rid of this.

This might be out of scope for this PR though, we can do it in a follow-up PR.

Copy link
Member Author

@maqi maqi Nov 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was to confirm a quote given by other node are not faked.

Sry, I don't mean faked by other, I mean faked by the quoter itself .
i.e. a node gives a quote with out_of_sequence data to gaim higher chance to be picked as a payee, or got a overpay by the client

};

let time_diff = old_elapsed.as_secs().saturating_sub(new_elapsed.as_secs());
Expand All @@ -275,14 +287,6 @@ impl PaymentQuote {
old_quote.quoting_metrics.close_records_stored
);

// TODO: Double check if this applies, as this will prevent a node restart with same ID
if new_quote.quoting_metrics.received_payment_count
< old_quote.quoting_metrics.received_payment_count
{
info!("claimed received_payment_count out of sequence");
return false;
}

true
}
}
Expand Down
Loading
Loading